Skip to content

Add pull request template#1827

Merged
ann0see merged 9 commits intomasterfrom
PRTemplate
Jun 22, 2021
Merged

Add pull request template#1827
ann0see merged 9 commits intomasterfrom
PRTemplate

Conversation

@ann0see
Copy link
Copy Markdown
Member

@ann0see ann0see commented Jun 4, 2021

Short description of changes

Added a pull request template to make contribution and maintenance easier.

Context: Fixes an issue?

This might make categorizing Pull Requests easier and help contributors and maintainers to grasp the state of a Pull Request faster.

Does this change need documentation? What needs to be documented and how?

No.

Status of this pull request

Waiting on second approval

What is missing until this pull request can be merged?

One missing approval of the content of the Template.
Squash and merge

Checklist

  • I verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.

@ann0see ann0see added this to the Release 3.8.1 milestone Jun 12, 2021
@gilgongo
Copy link
Copy Markdown
Member

gilgongo commented Jun 12, 2021

Good idea about reminding people about documentation :-) Ideally, it would also be be good to have people summarise the current status of the pull request if there's a lot of discussion or amends on it. For example, this IPv6 PR looks great, until you scroll though about 40 comments to see this #1017 (comment)

@ann0see
Copy link
Copy Markdown
Member Author

ann0see commented Jun 12, 2021

Yes ;-). There are multiple PRs like that.

@dcorson-ticino-com
Copy link
Copy Markdown
Contributor

Do you want to add to the checklist the semiauto changelog entry that was suggested ?

@ann0see
Copy link
Copy Markdown
Member Author

ann0see commented Jun 12, 2021

Sorry, I didn’t quite get what you mean. The checklist is to make reviewing easier and to have a certain quality standard as soon as possible.

The automatic change log script might get the content from the mentioned entry. Sure.

@dcorson-ticino-com
Copy link
Copy Markdown
Contributor

I mean to set a reminder in the template to not forget to include the line for the changelog entry, maybe as one of the things mentioned in the checklist.

@ann0see
Copy link
Copy Markdown
Member Author

ann0see commented Jun 13, 2021

Changed the headline to bold style (this makes them less prominent)

Copy link
Copy Markdown
Member

@hoffie hoffie left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me! Posted some suggestions and I think this is something which can be further improved on after we have observed for some time how it works in practice.

(I think I've tried adding a PR template and failed for some reason, not sure why)

Comment thread .github/pull_request_template.md Outdated
Comment thread .github/pull_request_template.md
Comment thread .github/pull_request_template.md
- [ ] I verified that this Pull Request follows the [general code principles](https://github.com/jamulussoftware/jamulus/blob/master/CONTRIBUTING.md#jamulus-projectsource-code-general-principles)
- [ ] I tested my code and it does what I want
- [ ] My code follows the [style guide](https://github.com/jamulussoftware/jamulus/blob/master/CONTRIBUTING.md#source-code-consistency) <!-- You can also check if your code passes clang-format -->
- [ ] I waited some time after this Pull Request was opened and all GitHub checks completed without errors. <!-- GitHub doesn't run these checks for new contributors automatically. -->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- [ ] I waited some time after this Pull Request was opened and all GitHub checks completed without errors. <!-- GitHub doesn't run these checks for new contributors automatically. -->
- [ ] I've waited some time after this Pull Request was opened and all GitHub checks completed without errors. <!-- GitHub doesn't run these checks for new contributors automatically. If nobody main developer has approved the checks after a few days, feel free to post a quick ping. -->

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If nobody main developer has approved the checks after a few days, feel free to post a quick ping

This might add pressure. Pinging should not be too common in my opinion.

Comment thread .github/pull_request_template.md Outdated
Comment thread .github/pull_request_template.md Outdated
ann0see and others added 4 commits June 13, 2021 22:00
Co-authored-by: Christian Hoffmann <mail@hoffmann-christian.info>
Co-authored-by: Christian Hoffmann <mail@hoffmann-christian.info>
Co-authored-by: Christian Hoffmann <mail@hoffmann-christian.info>
Co-authored-by: Christian Hoffmann <mail@hoffmann-christian.info>
- [ ] I tested my code and it does what I want
- [ ] My code follows the [style guide](https://github.com/jamulussoftware/jamulus/blob/master/CONTRIBUTING.md#source-code-consistency) <!-- You can also check if your code passes clang-format -->
- [ ] I waited some time after this Pull Request was opened and all GitHub checks completed without errors. <!-- GitHub doesn't run these checks for new contributors automatically. -->
- [ ] I've filled all the content above
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@dcorson-ticino-com would this wording be ok?

@ann0see
Copy link
Copy Markdown
Member Author

ann0see commented Jun 21, 2021

@gilgongo sorry for the ping here. Could you please also have a look at this? I think we should merge this PR soon.

Copy link
Copy Markdown
Member

@gilgongo gilgongo left a comment

Choose a reason for hiding this comment

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

Seems good to me - we can always modify it if things need changing.

@ann0see ann0see merged commit 1796a98 into master Jun 22, 2021
@ann0see ann0see deleted the PRTemplate branch June 22, 2021 15:20
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.

5 participants