Skip to content
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

Update docs such that they can be used in knative/docs #460

Merged
merged 1 commit into from
Feb 2, 2019

Conversation

bobcatfish
Copy link
Collaborator

In version 0.4 of knative, we'd like to release build-pipeline, which
means that the docs repo (https://github.com/knative/docs) should talk
about build-pipeline.

I think it's important to make sure that docs are updated with the code,
so I propose we have living versions of these docs in this repo, and
every time we release, we copy the docs into the knative/docs repo, so
that this repo has the correct docs at HEAD and knative/docs has the
correct docs as of the latest release. (How we want to deal with docs
for previous releases will have to be another story!)

In this PR, I've gone through the knative docs for
build
and shaped our
existing docs to be similar, which meant:

  • Taking content out of using.md and splitting it into a file for each
    type of CRD
  • Adding reference docs for the CRD fields (fixes Create reference docs #193)
  • Most of concepts.md and some of using.md became the README file which
    lives at the top level of the docs dir here and hopefully the new
    pipelines dir in knative/docs

This also meant gaining a lot of docs by taking existing Build docs and
updating them to talk about pipelines (e.g. docs about auth)

Fixes #304

Reviewers: I've probably missed some link, some references to Build, and some k8s standards. Your nits are very welcome!

(After we finish and merge this PR, I'll open a PR in knative/docs to add these docs there. If they request doc changes I'll backport them here)

@googlebot googlebot added the cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit label Feb 1, 2019
@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 1, 2019
@knative-prow-robot knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 1, 2019
Copy link
Contributor

@shashwathi shashwathi 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 awesome work @bobcatfish 👍

@knative-prow-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bobcatfish, shashwathi

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:
  • OWNERS [bobcatfish,shashwathi]

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

Copy link
Member

@nader-ziada nader-ziada left a comment

Choose a reason for hiding this comment

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

@bobcatfish awesome docs, thank you for doing that! 🎉

I left a few comments about some links not working, let me know if you want to get this in and iterate on the comments

- Optional:
- [`resources`](#declared-resources) - Specifies which [`PipelineResources`](resources.md)
of which types the `Pipeline` will be using in its [Tasks](#pipeline-tasks)
- [`timeout`](#timeout) - Specifies timeout after which the `Pipeline` will fail.
Copy link
Member

Choose a reason for hiding this comment

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

this link doesn't go anywhere

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😅

name: build-push
```

[Declared `PipelineResources`](#declared-resources) can be given to `Task`s in the `Pipeline` as
Copy link
Member

Choose a reason for hiding this comment

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

This link doesn't go anywhere

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmmm I think this one works 🤔

The resource `my-image` is expected to be given to the `deploy-app` `Task` from
the `build-app` `Task`. This means that the `PipelineResource` `my-image` must
also be declared as an output of `build-app`.

Copy link
Member

Choose a reason for hiding this comment

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

is that the same as build-the-image in the example above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

whoa this block seems completely out of place 😊

the `build-app` `Task`. This means that the `PipelineResource` `my-image` must
also be declared as an output of `build-app`.

For implementation details, see [the developer docs](docs/developers/README.md).
Copy link
Member

Choose a reason for hiding this comment

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

this link gives a 404 😲

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

whoops!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

im actually gonna remove this b/c the developer dir won't be copied over to knative/docs...

docs/tasks.md Outdated
authentication information.
- [`volumes`](#volumes) - Specifies one or more volumes that you want to make
available to your build.
- [`timeout`](#timeout) - Specifies timeout after which the `Pipeline` will fail.
Copy link
Member

Choose a reason for hiding this comment

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

This link doesn't go anywhere

docs/tasks.md Outdated
- [`inputs`](#inputs) - Specifies parameters and [`PipelineResources`](resources.md)
needed by your `Task`
- [`outputs`](#outputs) - Specifies [`PipelineResources`](resources.md) needed by your `Task`
- [`serviceAccount`](#service-account) - Specifies a `ServiceAccount`
Copy link
Member

Choose a reason for hiding this comment

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

This link doesn't go anywhere

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

whoops, thats only in taskrun! 😇

docs/tasks.md Outdated
A `Task` can declare the inputs it needs, which can be either or both of:

* [`parameters`](#parameters)
* [`input resources](#input-resources)
Copy link
Member

Choose a reason for hiding this comment

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

extra character before the word input

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😅

docs/tasks.md Outdated

- [Example of image building and pushing](#example-task)
- [Mounting extra volumes](#using-an-extra-volume)
- [Authenticating with `ServiceAccount`](#using-a-serviceaccount)
Copy link
Member

Choose a reason for hiding this comment

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

this link doesn't go anywhere

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😅

value: "world"
```

_See [the installation guide](installing.md) if you would like to
Copy link
Member

Choose a reason for hiding this comment

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

both links here give 404

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh yeah, we won't have that till we have #385 🤔

In version 0.4 of knative, we'd like to release build-pipeline, which
means that the docs repo (https://github.com/knative/docs) should talk
about build-pipeline.

I think it's important to make sure that docs are updated with the code,
so I propose we have living versions of these docs in this repo, and
every time we release, we copy the docs into the knative/docs repo, so
that this repo has the correct docs at HEAD and knative/docs has the
correct docs as of the latest release. (How we want to deal with docs
for previous releases will have to be another story!)

In this PR, I've gone through [the knative docs for
build](https://github.com/knative/docs/tree/master/build) and shaped our
existing docs to be similar, which meant:

* Taking content out of using.md and splitting it into a file for each
  type of CRD
* Adding reference docs for the CRD fields (fixes tektoncd#193)
* Most of concepts.md and some of using.md became the README file which
  lives at the top level of the `docs` dir here and hopefully the new
  `pipelines` dir in knative/docs

This also meant gaining a lot of docs by taking existing Build docs and
updating them to talk about pipelines (e.g. docs about auth)

Fixes tektoncd#304
@bobcatfish
Copy link
Collaborator Author

Thanks for noticing all those details @pivotal-nader-ziada !! Ready for another look :D

@nader-ziada
Copy link
Member

/test pull-knative-build-pipeline-integration-tests

@nader-ziada
Copy link
Member

looks good 👍

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 2, 2019
@knative-prow-robot knative-prow-robot merged commit c579a78 into tektoncd:master Feb 2, 2019
piyush-garg pushed a commit to piyush-garg/pipeline that referenced this pull request Jul 14, 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. cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update knative/docs with build-pipeline docs Create reference docs
5 participants