Skip to content

Refine the document for getting start #394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 22, 2020
Merged

Refine the document for getting start #394

merged 3 commits into from
Sep 22, 2020

Conversation

zhangtbj
Copy link
Contributor

@zhangtbj zhangtbj commented Sep 16, 2020

For requirement: https://kubernetes.slack.com/archives/C019ZRGUEJC/p1600182959018100

  • Add Want to contribute block more in the main README
  • Add more install and deployment related info in deploying.md
  • Change deploying.md to the DEVELOPMENT.md
  • Remove the Features block in the main README because it is already out-of-date

Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DEVELOPMENT.MD file is a copy from Tekton Pipelines, https://github.com/tektoncd/pipeline/blob/master/DEVELOPMENT.md plus adjustments for the build project. Please make appropriate changes to the copyright header of the file to comply with article 4 of the Apache License.

@zhangtbj
Copy link
Contributor Author

Hi Sascha,

What should I add in the header?

I also view other DEVELOPMENT.md, some of them are similar, such as knative/serving md:
https://github.com/knative/serving/blob/master/DEVELOPMENT.md

They all follow the similar way.

I am not sure if we also need to add the other license for the md format?

@SaschaSchwarze0
Copy link
Member

Hi Sascha,

What should I add in the header?

I also view other DEVELOPMENT.md, some of them are similar, such as knative/serving md:
https://github.com/knative/serving/blob/master/DEVELOPMENT.md

They all follow the similar way.

I am not sure if we also need to add the other license for the md format?

I would change the file header to:

<!--
Copyright 2018, 2020 The Tekton Authors
Copyright The Shipwright Contributors

SPDX-License-Identifier: Apache-2.0

Documentation inspired from https://github.com/tektoncd/pipeline/blob/ce7591acec8a6aa726d88e5cc057588665881ace/DEVELOPMENT.md
-->

Copy link
Member

@adambkaplan adambkaplan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great start and has a lot of good details that are missing in the existing docs.

I think with this and HACK.md we have two overlapping documents that IMO should target different audiences.

Others should weigh in, personally I'd like to see the following:

  1. A "Getting Started" guide to target an end-user who does not want to build from source. Perhaps we can simplify matters and assume the end user knows what Kubernetes is and has access to a cluster. The goal is to provide instructions on how to install Shipwright, create a Build, and start a BuildRun.
  2. A "Hacking" guide to target contributors - instructions on how to compile, test the operator locally, deploy your hacked version on a cluster, etc.

@zhangtbj
Copy link
Contributor Author

Hi Sascha,

I also correct the Copyright

DEVELOPMENT.md Outdated
Comment on lines 92 to 96
1. Set up a docker repository for pushing images. You can use any container
image registry by adjusting the authentication methods and repository paths
mentioned in the sections below.
- [Docker Hub quickstart](https://docs.docker.com/docker-hub/)
- [IBM Container Registry quickstart](https://cloud.ibm.com/docs/Registry)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would try to be a bit more neutral here:

Suggested change
1. Set up a docker repository for pushing images. You can use any container
image registry by adjusting the authentication methods and repository paths
mentioned in the sections below.
- [Docker Hub quickstart](https://docs.docker.com/docker-hub/)
- [IBM Container Registry quickstart](https://cloud.ibm.com/docs/Registry)
1. Set up a container image repository for pushing images. Any container image registry that is accessible to your cluster can be used for your repository. This can be a public registry like [Docker Hub])(https://docs.docker.com/docker-hub/), [quay.io](https://quay.io), or a container registry run by your cloud provider.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing. Done

@zhangtbj
Copy link
Contributor Author

Hi @SaschaSchwarze0 , @adambkaplan or all,

Do you have any suggestions?

If no other suggestion, I will go ahead to merge this PR.

Thanks!

@SaschaSchwarze0
Copy link
Member

SaschaSchwarze0 commented Sep 20, 2020 via email

@qu1queee
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 22, 2020
@qu1queee
Copy link
Contributor

/approve

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qu1queee

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 22, 2020
@openshift-merge-robot openshift-merge-robot merged commit 7fb59b5 into shipwright-io:master Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants