-
Notifications
You must be signed in to change notification settings - Fork 173
chore: update pull request template, add code review checklist #2043
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
themr0c
merged 38 commits into
eclipse-che:master
from
themr0c:chore-updating-pr-template
Aug 24, 2021
Merged
Changes from all commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
4badab0
chore: update pull request template, more complete review checklist, …
themr0c dbfa0b0
Fix language errors. Apply suggestions from reviewer.
themr0c 79e13fd
Merge branch 'master' into chore-updating-pr-template
MichalMaler 5f41347
Apply feedback from Community meeting
themr0c 83978f4
Merge branch 'master' into chore-updating-pr-template
MichalMaler 7931997
Merge branch 'master' into chore-updating-pr-template
MichalMaler 35163ae
Merge branch 'master' into chore-updating-pr-template
themr0c fe9cfc6
Merge branch 'master' into chore-updating-pr-template
MichalMaler 8851170
Removing "Describes a scenario already covered by QE tests, otherwise…
themr0c 777eb71
Make it a recommendation, not a blocker.
themr0c 68fb3b4
We do recongnize the subjectivity of some of these objectives.
themr0c 8af6d04
Update code_review_checklist.adoc
themr0c 870b03c
Update .github/PULL_REQUEST_TEMPLATE.md
themr0c 663ab22
Merge branch 'master' into chore-updating-pr-template
themr0c 2180763
Merge branch 'master' into chore-updating-pr-template
MichalMaler 07db950
Merge branch 'master' into chore-updating-pr-template
MichalMaler d1e25b0
Merge branch 'master' into chore-updating-pr-template
themr0c 3c6e8ac
Update .github/PULL_REQUEST_TEMPLATE.md
themr0c 8f6316f
Update .github/PULL_REQUEST_TEMPLATE.md
themr0c a1b57c8
Update .github/PULL_REQUEST_TEMPLATE.md
themr0c e9ab1db
Update .github/PULL_REQUEST_TEMPLATE.md
themr0c 59c2d6a
Update .github/PULL_REQUEST_TEMPLATE.md
themr0c 9e0af31
Update code_review_checklist.adoc
themr0c 98387ae
Update code_review_checklist.adoc
themr0c e5db42c
Update code_review_checklist.adoc
themr0c 5f3a40c
Update code_review_checklist.adoc
themr0c e26a7ca
Update code_review_checklist.adoc
themr0c 489d1a1
Update code_review_checklist.adoc
themr0c fa3928c
Update code_review_checklist.adoc
themr0c 62f9385
Merge branch 'master' into chore-updating-pr-template
themr0c 137f17b
Merge branch 'master' into chore-updating-pr-template
themr0c 94c4a4b
Update code_review_checklist.adoc
themr0c bbd16a3
Update location for branding.constant.ts
themr0c 25728a3
Consistency: pull request
themr0c cc167c3
Merge branch 'master' into chore-updating-pr-template
themr0c 1a358f1
Merge branch 'master' into chore-updating-pr-template
themr0c f43f2cf
Merge branch 'master' into chore-updating-pr-template
themr0c 8572b6b
Update .github/PULL_REQUEST_TEMPLATE.md
themr0c File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,30 @@ | ||
| > Read our [Contribution guide](https://github.com/eclipse/che-docs/blob/master/CONTRIBUTING.adoc) before submitting a PR. | ||
|
|
||
| ### What does this PR do? | ||
| <!-- | ||
| Please use one of the following prefixes for the title: | ||
| docs: Documentation not including procedures. Engineering review is mandatory. | ||
| procedures: Documentation including procedures. Testing procedures is mandatory. Engineering and QE review is mandatory (Engineering can review on behalf of QE). | ||
| chore: Routine, release, tooling, version upgrades. | ||
| fix: Fix build, language, links, or metadata. | ||
| --> | ||
|
|
||
| <!-- Read our [Contribution guide](https://github.com/eclipse/che-docs/blob/master/CONTRIBUTING.adoc) before submitting a PR. --> | ||
|
|
||
| ### What issues does this PR fix or reference? | ||
| ## What does this pull request change? | ||
|
|
||
| ## What issues does this pull request fix or reference? | ||
|
|
||
| ### Specify the version of the product this PR applies to. | ||
| ## Specify the version of the product this pull request applies to | ||
|
|
||
| ## Pull Request checklist | ||
|
|
||
| ### PR Checklist | ||
| The author and the reviewers validate the content of this pull request with the following checklist, in addition to the [automated tests](code_review_checklist.adoc). | ||
|
|
||
| As the author of this Pull Request I made sure that: | ||
|
|
||
| - [ ] `vale` has been run successfully against the PR branch | ||
| - [ ] Link checker has been run successfully against the PR branch | ||
| - [ ] Documentation describes a scenario that is already covered by QE tests, otherwise an issue has been created and acknowledged by Che QE team | ||
| - [ ] Changed article references are updated where they are used (or a redirect has been configured on the docs side): | ||
| - [ ] Dashboard [default branding data](https://github.com/eclipse-che/che-dashboard/blob/main/src/services/bootstrap/branding.constant.ts) | ||
| - Any procedure: | ||
| - [ ] Successfully tested. | ||
| - Any page or link rename: | ||
| - [ ] The page contains a redirection for the previous URL. | ||
| - Propagate the URL change in: | ||
| - [ ] Dashboard [default branding data](https://github.com/eclipse-che/che-dashboard/blob/main/packages/dashboard-frontend/src/services/bootstrap/branding.constant.ts) | ||
| - [ ] Chectl [constants.ts](https://github.com/che-incubator/chectl/blob/master/src/constants.ts) | ||
| - [ ] Builds on [Eclipse Che hosted by Red Hat](https://workspaces.openshift.com). | ||
| - [ ] the *`Validate language on files added or modified`* step reports no vale warnings. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| # Pull request review checklist for committers and reviewers | ||
| :toc: auto | ||
|
|
||
| The reviewer validates the content of a pull request with this checklist. | ||
|
|
||
| ## Automated validation steps | ||
|
|
||
| These automated steps block the pull request merge. | ||
|
|
||
| * [ ] *`Build and validate PR / link checker (pull_request)`* is successful | ||
| ** [ ] *`Build using Antora`* step is successful: documentation is building without warnings, and all AsciiDoc attributes have a definition. | ||
| ** [ ] *`Validate links using htmltest`* step is successful: internal and external links are valid. | ||
| ** [ ] *`Validate language on files added or modified`* step is successful and reports no vale errors: basic language validation, no major deviations from link:https://www.oreilly.com/library/view/the-ibm-style/9780132118989/[IBM Style Guide] and link:https://redhat-documentation.github.io/supplementary-style-guide/[Supplementary Style Guide], and project specific language rules. | ||
| * [ ] *`eclipsefdn/eca`* is successful: all contributors have signed the Eclipse Contributor Agreement (ECA). | ||
|
|
||
| ## Non automated validation steps | ||
|
|
||
| The author and the reviewers validate the content of a pull request with this additional checklist: | ||
|
|
||
| * Any procedure: | ||
| ** [ ] Successfully tested. | ||
| * Any page or link rename: | ||
| ** [ ] The page contains a redirection for the previous URL. | ||
| ** Propagate the URL change in: | ||
| *** [ ] Dashboard [default branding data](https://github.com/eclipse-che/che-dashboard/blob/main/src/services/bootstrap/branding.constant.ts) | ||
| *** [ ] Chectl [constants.ts](https://github.com/che-incubator/chectl/blob/master/src/constants.ts) | ||
| * [ ] Builds on https://workspaces.openshift.com[Eclipse Che hosted by Red Hat]. | ||
| * [ ] *`Validate language on files added or modified`* step reports no vale warnings. | ||
|
|
||
| ## In depth language review checklist | ||
themr0c marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| The technical writer reviewers validate the language with this additional in depth checklist. We do recognize the subjectivity of some of these objectives. | ||
|
|
||
| - [ ] Content is appropriate for the intended audience. | ||
| - [ ] Downstream friendly: correct use of attributes, `ifdef` statements, examples. | ||
| - [ ] Effective titles. | ||
| - [ ] Follows modularization guidelines. See: link:https://redhat-documentation.github.io/modular-docs/[Modular Documentation Reference Guide]. | ||
| - [ ] Follows the principles of minimalism. See: link:https://docs.google.com/presentation/d/1Yeql9FrRBgKU-QlRU-nblPJ9pfZKgoKcU8SW6SQ_UqI/edit#slide=id.g1f4790d380_2_257[The Wisdom of Crowds slides], link:https://youtu.be/s3Em8QSXyn8[The Wisdom of Crowds video], link:https://www.nngroup.com/articles/chunking/[Chunking]. | ||
| - [ ] Information is clear, valuable, and concise, no ambiguity. | ||
| - [ ] Language is consistent with the rest of the docs. | ||
| - [ ] Language is compliant with link:https://www.oreilly.com/library/view/the-ibm-style/9780132118989/[IBM Style Guide] and link:https://redhat-documentation.github.io/supplementary-style-guide/[Supplementary Style Guide] rules not implemented in vale. | ||
| - [ ] The information flow is logical. | ||
| - [ ] Uses screenshot and other visuals when required. | ||
| - [ ] Uses verification steps when necessary. | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.