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

contributing: update the commit guidelines #806

Merged
merged 1 commit into from
Jul 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 11 additions & 38 deletions .github/contributing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -87,50 +87,23 @@ Anatomy of a Pull Request
=========================

There is a GitHub Pull Request template to help guide crafting a description,
and you can liberally copy content from the commit message as needed.
and you can liberally copy content from the commit messages as needed.

If your PR isn't quite ready feel free to create it as a draft, then once
you're all set feel free to flip it to "Ready for review".

The PR can have as many commits as you want, with multiple commits being
preferred, as it can make review easier, especially when there are many
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would consider some more of the explanation from the PR description -- why not squash manually and other things mentioned there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, this is how we end up with hard to follow docs. The writing style should be, keep everything very brief and to the point upfront, then elaborate later on.

Most people just want the instructions to follow and get going. We don't have to defend each point, nor do we have to convince people, right at that moment. If a would be contributor demands/feels they need that for them to follow along then they can get fucked.

changes.

One Commit Per Pull Request
---------------------------

Pull request should contain a single **passing** commit. This is necessary
to ensure clean git history that is not cluttered by a partially working,
failing and outright failing to compile states.

To achieve this you can do either of the following:

- If the change fits into a single commit you don't need to do anything
- If you need to made some additional modifications (review requested) you
can amend the commit and force-push it (`git commit --amend
--no-edit`:cmd: and `git push --force <remote> <your branch>`:cmd:)
- Create multiple commits and then squash them when your pull request is
approved.
1. Create multiple commits
2. Create new branch based on `devel` (`git checkout devel`:cmd: and `git
checkout <branch>-squashed`:cmd:)
3. Squash merge your original branch into a new one - `git merge --squash
<branch>`:cmd:
4. Commit your squashed branch using `git commit`:cmd:

.. note::

By default you will get pre-filled commit message which contains
pretty verbose "sqash of the following" message - those are not
going to be accepted by the PR reviewers, and need to be edited
into human-readable error message according to the commit message
guidelines.

.. tip::

You can write a commit message as a PR description and then
copy-paste it when you are done with the implementation.

5. Force-push your squashed branch using `git push --force <remote>
HEAD:<branch>`:cmd:
When the PR is merged, a squashed version of the commits is automatically
created (leaving both the original branch and the commits as they show up in
the history on GitHub intact), with everything coming before the first section
break (`---`) in the PR message being used as the commit message.

Force pushing to a branch for which a PR has already been created is okay, but
please try to only rewrite history for commits that haven't been reviewed yet,
so as to make incremental reviews easier.

Commit Message
--------------
Expand Down
5 changes: 4 additions & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
<!--- The Pull Request (=PR) message is what will get automatically used as
the commit message when the PR is merged. Make sure that no line is longer
than 72 characters -->

## Summary
* what changed and how?
* why are we changing it?
Expand Down Expand Up @@ -30,4 +34,3 @@ for details, especially if you're new to this project.
Tips that make PRs easier:
* for big/impactful changes, start with chat/discussions to refine ideas
* refine the pull request message over time; don't have to nail it in one go
* handle the single commit message requirement at the end of review
Loading