Skip to content

Explicitly state on the PR template that we can squash commits #6662

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
merged 3 commits into from
Feb 3, 2020

Conversation

nicoddemus
Copy link
Member

This way we don't need to ask everytime, and users who for some reason
would not like us to squash their commits can uncheck the box then.

This way we don't need to ask everytime, and users who for some reason
would not like us to squash their commits can uncheck the box then.
@@ -20,4 +20,7 @@ Unless your change is trivial or a small documentation fix (e.g., a typo or rewo
Also make sure to end the sentence with a `.`.

- [ ] Add yourself to `AUTHORS` in alphabetical order.

If you leave the *Allow edits from maintainers* checked, the mainteiners might squash your commits before merging. If unchecked, maintainers
will probably ask you to squash yourself then.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is confusing.
squash-merging is not the same as (force-)pushing to the branch ("allow edits from maintainers").

might squash your commits before merging

This sounds like it would mean "squashed, force-pushed and then merged" (not squash-merged).

(Squash-merging is only an edit in the sense of maybe changing the commit message(, and having a single commit only of course.).)

So while a sentence like that is good, and the checkbox could be used as an indicator for that, it should be clarified.
I can think of it and make a suggestion in case my point does not come across clearly.. 👍

(also there's a typo at least: mainteiners)

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed, thanks. Feel free to make further suggestions directly. 👍

Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

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

👍

@nicoddemus nicoddemus merged commit 1480aa3 into pytest-dev:master Feb 3, 2020
@nicoddemus nicoddemus deleted the pr-template branch February 3, 2020 17:35
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.

3 participants