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

Refactoring of Testgrid Readme Files #13104

Merged
merged 1 commit into from Jun 21, 2019

Conversation

Projects
None yet
5 participants
@chases2
Copy link
Contributor

commented Jun 20, 2019

Breaks apart single "mono-file" to files targeted to particular users:

  • README.md is for users who want to use the website, or who don't know
    what they want
  • configuration.md is for users who want to change a configuration
  • build_test_update.md is for users who want to develop TestGrid

See Issue #13057

/assign @michelle192837
/cc @spiffxp

@chases2

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2019

/assign @wojtek-t

@spiffxp
Copy link
Member

left a comment

few nits, main one is config.md instead of configuration.md

* A list of dashboard groups of related dashboards.

## Tip and Tricks
The testgrid site is accessible at https://testgrid.k8s.io.

This comment has been minimized.

Copy link
@spiffxp

spiffxp Jun 21, 2019

Member
Suggested change
The testgrid site is accessible at https://testgrid.k8s.io.
The Kubernetes Testgrid instance is available at https://testgrid.k8s.io

Run `bazel run //hack:update-protos` to generate, and `bazel run //hack:verify-protos.sh`
to verify.
If you need to add a new test that you want TestGrid to display, see [Testgrid Configuration](configuration.md).

This comment has been minimized.

Copy link
@spiffxp

spiffxp Jun 21, 2019

Member

nit: how about making this config.md to parallel config.yaml?

This comment has been minimized.

Copy link
@spiffxp

spiffxp Jun 21, 2019

Member

nit: there are more uses cases than adding a new test

@@ -402,43 +101,17 @@ Under **Options**
vice versa) with more weight given to more recent transitions.
* **Sort by Name**: Sort alphabetically.

## Unit testing

Run `bazel test //testgrid/...` to ensure the config is valid.

This comment has been minimized.

Copy link
@spiffxp

spiffxp Jun 21, 2019

Member

nit: not clear to me whether this was intentionally deleted, I feel like it belongs in build_test_update.md

This comment has been minimized.

Copy link
@chases2

chases2 Jun 21, 2019

Author Contributor

It was moved to the configuration.md file; under #testing-your-configuration.

It seemed that someone changing a configuration who might not be familiar with "bazel test" at all would need to know about this test function.

This comment has been minimized.

Copy link
@spiffxp

spiffxp Jun 21, 2019

Member

got it; at present I'd still prefer it in build_test_update, the configuration file seems like a reference doc, not a how-to/playbook, but if others feel strongly the other way I'll let it drop

This comment has been minimized.

Copy link
@chases2

chases2 Jun 21, 2019

Author Contributor

I included a Testing section that lists the command here, but doesn't go into detail into its effects on config.yaml so that the doc isn't repeating itself.

Hopefully the best of both worlds

@spiffxp

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

Doesn't look like this breaks any incoming links from within our orgs, so looks good from that perspective https://cs.k8s.io/?q=%2Ftestgrid%5B%5E%5C.%5D&i=nope&files=&repos=

Refactoring of Testgrid Readme Files
Breaks apart single "mono-file" to files targeted to particular users:
- README.md is for users who want to use the website, or who don't know
what they want
- configuration.md is for users who want to change a configuration
- build_test_update.md is for users who want to develop TestGrid

See Issue #13057

@chases2 chases2 force-pushed the chases2:testgrid-automation branch from 71563cb to 2aa6793 Jun 21, 2019

@michelle192837

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2019

/lgtm
/approve
/hold

Leaving it held in case @spiffxp has more to say, but looks good to me!

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2019

LGTM label has been added.

Git tree hash: 6f13b22b6b1161077de8b2326f385c849edda521

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chases2, michelle192837

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

@spiffxp

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

/hold cancel
thanks for doing this!

@k8s-ci-robot k8s-ci-robot merged commit a2a1455 into kubernetes:master Jun 21, 2019

6 checks passed

cla/linuxfoundation chases2 authorized
Details
pull-test-infra-bazel Job succeeded.
Details
pull-test-infra-gubernator Skipped.
pull-test-infra-verify-file-perms Job succeeded.
Details
pull-test-infra-yamllint Job succeeded.
Details
tide In merge pool.
Details

@chases2 chases2 deleted the chases2:testgrid-automation branch Jun 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.