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

tools: Update Github pull request template #524

Merged
merged 4 commits into from
Jun 14, 2022

Conversation

orpheuslummis
Copy link
Contributor

@orpheuslummis orpheuslummis commented Jun 14, 2022

RELEVANT ISSUE(S)

Resolves #523

DESCRIPTION

Update pull request template with the following goals:

  • simplification and clarity
  • making sure the PR author checks all the boxes
  • nudging the PR author to discuss limitations

Starting as a draft PR for the purpose of reaching an happy consensus.

HOW HAS THIS BEEN TESTED?

Not applicable.

CHECKLIST:

  • I have commented the code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the repo-held documentation.
  • I have made sure that the PR title adheres to the conventional commit style (subset of the ones we use can be found under: tools/configs/chglog/config.yml

ENVIRONMENT / OS THIS WAS TESTED ON?

Please specify which of the following was this tested on (remove or add your own):

  • Arch Linux
  • Debian Linux
  • MacOS
  • Windows

@orpheuslummis orpheuslummis added meta/dev These issues related to the meta elements of the code base like dev flow, repo logistics, and PM action/no-benchmark Skips the action that runs the benchmark. labels Jun 14, 2022
@orpheuslummis orpheuslummis added this to the DefraDB v0.3 milestone Jun 14, 2022
@orpheuslummis orpheuslummis self-assigned this Jun 14, 2022
@orpheuslummis orpheuslummis requested a review from a team June 14, 2022 03:56
@orpheuslummis
Copy link
Contributor Author

orpheuslummis commented Jun 14, 2022

@orpheuslummis
Copy link
Contributor Author

In the suggested template, do you you think you would usually check all boxes ?

@shahzadlone
Copy link
Member

In the suggested template, do you you think you would usually check all boxes ?

Most likely not, because I don't always have to update the repo held documentation for example. Unless maybe the wording was like if applicable, you updated repo held docs.....
Not sure if we want to go that route though.

@AndrewSisley
Copy link
Contributor

In the suggested template, do you you think you would usually check all boxes ?

I usually do, e.g. even if I think the code requires no comments, I see the code as properly documented and tick it off. The number of done/todo tasks are visible at the top of the PR and is a very good way of telling if there is anything left outstanding (the OS boxes mess with this a bit, and I dont like them, but as long as there are only 3 remaining tasks on a PR then I will merge after approval)

@orpheuslummis
Copy link
Contributor Author

In the suggested template, do you you think you would usually check all boxes ?

Most likely not, because I don't always have to update the repo held documentation for example. Unless maybe the wording was like if applicable, you updated repo held docs..... Not sure if we want to go that route though.

I think it'd be important, somehow, for PR authors to be lead into "checking all the tasks/boxes"

@orpheuslummis orpheuslummis marked this pull request as ready for review June 14, 2022 17:24
@orpheuslummis orpheuslummis merged commit a1bd3d7 into develop Jun 14, 2022
@orpheuslummis orpheuslummis deleted the orpheus/tools/pr-template-update branch June 14, 2022 17:26
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/no-benchmark Skips the action that runs the benchmark. meta/dev These issues related to the meta elements of the code base like dev flow, repo logistics, and PM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update PR template for clarity and discussion of limitations
3 participants