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

Improve Prow development docs a bit. #12245

Merged
merged 1 commit into from Apr 18, 2019
Merged

Conversation

cjwagner
Copy link
Member

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/prow Issues or PRs related to prow area/prow/phony Issues or PRs related to prow's phony component sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Apr 17, 2019
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 17, 2019
```shell
# export PROW_REPO_OVERRIDE=gcr.io/k8s-prow # optionally override project
push.sh # Build and push the current repo state.
bump.sh --list # Choose a recent published version
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only part of the "How to update the cluster" section that I actually updated. The rest I just moved.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this using @Katharine 's new tool?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, we just moved the ./bump.sh --push functionality into the ./push.sh script AFAICT.

Copy link
Contributor

@stevekuznetsov stevekuznetsov left a comment

Choose a reason for hiding this comment

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

Sweet, love the updates

**TODO**(cjwagner): Unify and document how to run prow components locally.
### How to test a plugin

If you are making changes to a Prow plugin you can test the new behavior by sending fake webhook events with [`phony`](/prow/cmd/phony#phony).
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe

sending fake webhooks to `hook` with `phony`

?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

```shell
# export PROW_REPO_OVERRIDE=gcr.io/k8s-prow # optionally override project
push.sh # Build and push the current repo state.
bump.sh --list # Choose a recent published version
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this using @Katharine 's new tool?

bazel run //prow/cluster:hook.apply # just update hook

# This is equivalent to doing the following with kubectl directly:
kubectl config use-context gke_my-project_my-zone_my-cluster
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we de-GKE-ify this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. What do you suggest?

Copy link
Member Author

Choose a reason for hiding this comment

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

I switched to my-k8s-cluster-context

prow/jobs.md Outdated
Jobs that run will always post a status context to the commit under test in GitHub.
Jobs that run conditionally but do not match the content of the pull request will
_not_ post "Skipped" status contexts to the pull request.
Presubmits that do not have `skip_report: true` will always post a status context to the commit under test in GitHub.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: text wrapping length

Copy link
Contributor

Choose a reason for hiding this comment

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

Technically now postsubmits can publish

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we flip the negative here to have the common case first?

Jobs will post a status context in GitHub to the commit under test, unless the job is configured with `skip_report: true`

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

The text wrapping still displays correctly if we don't manually manage the line length. I'm not a fan of shuffling paragraph text around when I add a few words to the first sentence so whenever I touch markdown text that manually manages line length I switch it to long lines and let GH manage the wrapping.

@stevekuznetsov
Copy link
Contributor

/lgtm
/approve
/woof
/meow

@k8s-ci-robot
Copy link
Contributor

@stevekuznetsov: dog image

In response to this:

/lgtm
/approve
/woof
/meow

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot
Copy link
Contributor

@stevekuznetsov: cat image

In response to this:

/lgtm
/approve
/woof
/meow

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 18, 2019
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 2f8412c50efe6df74b67ca88e47c03280b1e171d

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cjwagner, stevekuznetsov

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

@k8s-ci-robot k8s-ci-robot merged commit abcd35c into kubernetes:master Apr 18, 2019
@cjwagner cjwagner deleted the docs branch May 8, 2019 22:13
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. area/prow/phony Issues or PRs related to prow's phony component area/prow Issues or PRs related to prow cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants