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

Clarify code review checklist instructions #12056

Merged
merged 2 commits into from
Feb 12, 2021
Merged

Conversation

feerrenrut
Copy link
Contributor

Link to issue number:

Fixes #12054

Summary of the issue:

Instructions for the how to use and the intentions of the code review checklist were not clear enough.

Description of how this pull request fixes the issue:

Clarifies the instructions.
The intent of the checklist is to serve purely as a reminder, checking items communicates that you have considered the item, and have not accidentally forgotten about it.
Further, the checklist acts as a reminder for reviewers, unfortunately GitHub does not yet provide a way to create a dedicated code review template.

Additionally the wiki page has been adjusted: https://github.com/nvaccess/nvda/wiki/Github-pull-request-template-explanation-and-examples

Testing strategy:

This PR serves as an example.

Known issues with pull request:

None

Change log entry:

N/A

Code Review Checklist:

This checklist is a reminder of things commonly forgotten in a new PR.
Authors, please do a self-review and confirm you have considered the following items.
Mark items you have considered by checking them.
You can do this when editing the Pull request description with an x: [ ] becomes [x].
You can do also check the checkboxes after the PR is created.

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

@feerrenrut
Copy link
Contributor Author

@CyrilleB79 please take a look at this change.

CyrilleB79
CyrilleB79 previously approved these changes Feb 10, 2021
Copy link
Collaborator

@CyrilleB79 CyrilleB79 left a comment

Choose a reason for hiding this comment

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

Thank you Reef. Your intent has been clarified.
Let's then see how it is used by PR authors and reviewers.

.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
Co-authored-by: Cyrille Bougot <[email protected]>
@feerrenrut feerrenrut merged commit e5d62e3 into master Feb 12, 2021
@feerrenrut feerrenrut deleted the addCodeReveiwChecklist branch February 12, 2021 06:48
@nvaccessAuto nvaccessAuto added this to the 2021.1 milestone Feb 12, 2021
@XLTechie
Copy link
Collaborator

XLTechie commented Feb 13, 2021 via email

@feerrenrut
Copy link
Contributor Author

Oops. Yes, small typo there. Happy to accept a PR to correct!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify how to fill the code Review Checklist:
4 participants