Closed Bug 1406383 Opened 3 years ago Closed 3 years ago

DevTools: Document how to squash commits (for contributors without access to mozreview)

Categories

(DevTools :: General, enhancement, P3)

enhancement

Tracking

(firefox57 fix-optional, firefox58 fix-optional, firefox59 fixed)

RESOLVED FIXED
Firefox 59
Tracking Status
firefox57 --- fix-optional
firefox58 --- fix-optional
firefox59 --- fixed

People

(Reporter: sole, Assigned: pradeepgangwar)

Details

(Whiteboard: [dt-docs])

Attachments

(1 file)

We often ask contributors to 'squash their commits'. We should provide the commands to do so, and not expect them to know already.

These commands should be added for both git and hg, in the page that describes how to make patches: devtools/docs/contributing/making-prs.md

The page is online at http://docs.firefox-dev.tools/contributing/making-prs.html
Severity: normal → enhancement
Priority: -- → P3
I have used `histedit` in mercurial and `git rebase -i` . But as there are more tools available for achieving same results, is it ok to include only these two as examples in docs?
Flags: needinfo?(spenades)
Hello Pradeep, and sorry for the slow reply as I was away.

If you can provide syntax examples for these two, it's a good start. If someone wants to add alternatives, that's good. We don't need to be exhaustive here :)

Looking forward to your patch!

Thanks
Flags: needinfo?(spenades)
Recently, I have worked with someone who was using Mercurial and mq, and I couldn't remember the procedure for that setup...  

So, that would be another style to consider adding to this doc (in case it needs different steps from regular Mercurial).
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #3)
> Recently, I have worked with someone who was using Mercurial and mq, and I
> couldn't remember the procedure for that setup...  
> 
> So, that would be another style to consider adding to this doc (in case it
> needs different steps from regular Mercurial).

There's a bit of documentation on that here: https://developer.mozilla.org/en-US/docs/Mozilla/Mercurial/Queues#Folding_multiple_patches_into_one.
As a side note, with git, to find the base commit to call `git rebase -i`, I use `git merge-base upstream/master HEAD` (if "upstream" is upstream's remote, and "master" the branch we track). Maybe there's a better command to use the tracked branch more automatically.
Comment on attachment 8938703 [details]
Bug 1406383- Add documentation for squashing commits;

https://reviewboard.mozilla.org/r/209292/#review218214

Hello Pradeep - apologies for the delay in reviewing as I was on holidays.

Thank you so much for the contribution!

I made a very thorough review that I think will totally make this a great doc for new contributors to the code. Many many thanks for seeking out this information and taking the care of adding it to our docs ^_^

::: devtools/docs/contributing/making-prs.md:31
(Diff revision 1)
> +
> +## Squashing commits
> +
> +Sometimes you may be asked to squash your commits. Squashing means merging multiple commits into one in case you created multiple commits while working on a bug. Squashing bugs is easy with steps listed below for both git and mercurial.
> +
> +#### With Mercurial:

Use the correct level (### three, not #### four)

::: devtools/docs/contributing/making-prs.md:33
(Diff revision 1)
> +
> +Sometimes you may be asked to squash your commits. Squashing means merging multiple commits into one in case you created multiple commits while working on a bug. Squashing bugs is easy with steps listed below for both git and mercurial.
> +
> +#### With Mercurial:
> +
> +We will use histedit extension for squashing commits in mercurial. You can check if this extension is added in your mercurial following these steps:

*the* `histedit`

Mercurial should be uppercase

Also I guess it would sound better if it read: "You can check if this extension is enabled in your Mercurial configuration..."

But I don't know if you enable or install it - please change 'enabled' to 'installed' if you have to install it :)

::: devtools/docs/contributing/making-prs.md:34
(Diff revision 1)
> +Sometimes you may be asked to squash your commits. Squashing means merging multiple commits into one in case you created multiple commits while working on a bug. Squashing bugs is easy with steps listed below for both git and mercurial.
> +
> +#### With Mercurial:
> +
> +We will use histedit extension for squashing commits in mercurial. You can check if this extension is added in your mercurial following these steps:
> +- Open `.hgrc` (default configuration file of mercurial) located in home directory on Linux/OSX, using your favourite editor. 

in *the* home directory

::: devtools/docs/contributing/making-prs.md:34
(Diff revision 1)
> +Sometimes you may be asked to squash your commits. Squashing means merging multiple commits into one in case you created multiple commits while working on a bug. Squashing bugs is easy with steps listed below for both git and mercurial.
> +
> +#### With Mercurial:
> +
> +We will use histedit extension for squashing commits in mercurial. You can check if this extension is added in your mercurial following these steps:
> +- Open `.hgrc` (default configuration file of mercurial) located in home directory on Linux/OSX, using your favourite editor. 

Please replace the hyphen '-' with * to get bullet points

::: devtools/docs/contributing/making-prs.md:35
(Diff revision 1)
> +
> +#### With Mercurial:
> +
> +We will use histedit extension for squashing commits in mercurial. You can check if this extension is added in your mercurial following these steps:
> +- Open `.hgrc` (default configuration file of mercurial) located in home directory on Linux/OSX, using your favourite editor. 
> +- Then add `histedit= ` under `[exensions]` list present in file, if not present already.

Replace '-' with '*'

Also you probably meant `exTensions`, not `exensions`?

And it should be under "the [extensions]` list"

::: devtools/docs/contributing/making-prs.md:37
(Diff revision 1)
> +
> +We will use histedit extension for squashing commits in mercurial. You can check if this extension is added in your mercurial following these steps:
> +- Open `.hgrc` (default configuration file of mercurial) located in home directory on Linux/OSX, using your favourite editor. 
> +- Then add `histedit= ` under `[exensions]` list present in file, if not present already.
> +
> +After you have verified histedit follow these commands:

Replace "After you have verified histedit" with "Then, run the following command:"

::: devtools/docs/contributing/making-prs.md:38
(Diff revision 1)
> +We will use histedit extension for squashing commits in mercurial. You can check if this extension is added in your mercurial following these steps:
> +- Open `.hgrc` (default configuration file of mercurial) located in home directory on Linux/OSX, using your favourite editor. 
> +- Then add `histedit= ` under `[exensions]` list present in file, if not present already.
> +
> +After you have verified histedit follow these commands:
> +* `hg histedit`

Use the proper code formatting syntax and remove the bullet point

```bash
hg histedit
```

::: devtools/docs/contributing/making-prs.md:40
(Diff revision 1)
> +- Then add `histedit= ` under `[exensions]` list present in file, if not present already.
> +
> +After you have verified histedit follow these commands:
> +* `hg histedit`
> +
> +You will see this on your terminal editor:

- replace "see this" with "something like this"
- remove "editor"

::: devtools/docs/contributing/making-prs.md:47
(Diff revision 1)
> +```
> +pick 3bd22d1cc59a 0 "First-Commit-Message"
> +pick 81c4d40e57d3 1 "Second-Commit-Message"
> +```
> +
> +* These lines represent your commits. Suppose we want to merge 81c4d40e57d3 to 3bd22d1cc59a. Then replace **pick** in front of 81c4d40e57d3 with **fold** (or simply 'f'). Save the channges in your terminal.

Remove the bullet point, these are just paragraphs and not a list of related items.

::: devtools/docs/contributing/making-prs.md:47
(Diff revision 1)
> +```
> +pick 3bd22d1cc59a 0 "First-Commit-Message"
> +pick 81c4d40e57d3 1 "Second-Commit-Message"
> +```
> +
> +* These lines represent your commits. Suppose we want to merge 81c4d40e57d3 to 3bd22d1cc59a. Then replace **pick** in front of 81c4d40e57d3 with **fold** (or simply 'f'). Save the channges in your terminal.

channges -> changes

Remove 'in your terminal'

::: devtools/docs/contributing/making-prs.md:49
(Diff revision 1)
> +pick 81c4d40e57d3 1 "Second-Commit-Message"
> +```
> +
> +* These lines represent your commits. Suppose we want to merge 81c4d40e57d3 to 3bd22d1cc59a. Then replace **pick** in front of 81c4d40e57d3 with **fold** (or simply 'f'). Save the channges in your terminal.
> +
> +* You will see that 81c4d40e57d3 has been combined with 3bd22d1cc59a. You can verify this using `hg log` command.

Remove the bullet point

::: devtools/docs/contributing/making-prs.md:51
(Diff revision 1)
> +
> +* These lines represent your commits. Suppose we want to merge 81c4d40e57d3 to 3bd22d1cc59a. Then replace **pick** in front of 81c4d40e57d3 with **fold** (or simply 'f'). Save the channges in your terminal.
> +
> +* You will see that 81c4d40e57d3 has been combined with 3bd22d1cc59a. You can verify this using `hg log` command.
> +
> +* You can use fold to as many commits you want and they will be combined with the first commit above them which does not use fold.

Remove the bullet point

::: devtools/docs/contributing/making-prs.md:53
(Diff revision 1)
> +
> +* You will see that 81c4d40e57d3 has been combined with 3bd22d1cc59a. You can verify this using `hg log` command.
> +
> +* You can use fold to as many commits you want and they will be combined with the first commit above them which does not use fold.
> +
> +#### With Git:

Use ### and not ####

::: devtools/docs/contributing/making-prs.md:55
(Diff revision 1)
> +
> +* You can use fold to as many commits you want and they will be combined with the first commit above them which does not use fold.
> +
> +#### With Git:
> +
> +To squash commits in git follow these steps:

Replace with:

To squash commits in git we will use the rebase command. Let's learn the syntax using an example:

::: devtools/docs/contributing/making-prs.md:56
(Diff revision 1)
> +* You can use fold to as many commits you want and they will be combined with the first commit above them which does not use fold.
> +
> +#### With Git:
> +
> +To squash commits in git follow these steps:
> +* `git rebase -i HEAD~2`

Use proper formatting, also remove the bullet point:

```bash
git rebase -i HEAD~2
```

::: devtools/docs/contributing/making-prs.md:58
(Diff revision 1)
> +#### With Git:
> +
> +To squash commits in git follow these steps:
> +* `git rebase -i HEAD~2`
> +
> +Above commands is for rebasing commits, -i flag says run rebase in interactive mode and HEAD~2 says we need to rebase last 2 commits. You can use any number according to commits you want to rebase. This would open following on your terminal screen.

It would read better as:

* The -i flag is to run rebase in interactive mode
* HEAD~2 is to rebase the last two commits

You can use any number, depending on how many commits you want to rebase.

After you run that command, an editor will open displaying contents similar to this:

::: devtools/docs/contributing/making-prs.md:65
(Diff revision 1)
> +```
> +pick 5878025 "First-Commit-Message"
> +pick bd1efe7 "Second-Commit-Message"
> +```
> +
> +* Suppose you want to merge bd1efe7 to 5878025, then replace **pick** with **squash** (or simply 's') and save the changes, then a followup terminal screen should open like this

Remove the bullet point.

It's also important to use formatting around the hashes to make them stand up: `bd1efe7` rather than just bd1efe7.

I guess after 'save the changes' you should also close the editor?

And is it a follow up terminal screen that opens or another file to be edited?

::: devtools/docs/contributing/making-prs.md:79
(Diff revision 1)
> +
> +    < Second-Commit-Message >
> +
> +```
> +
> +* Write a new commit message (note that this will be a new commit message used for sqashed commits). Every line not starting with # above, contributes to commit message.

sqUashed, not squashed :-) 

Also remove the bullet point and the comma before "contributes"

Add 'the' before commit: "to the commit message"

::: devtools/docs/contributing/making-prs.md:80
(Diff revision 1)
> +    < Second-Commit-Message >
> +
> +```
> +
> +* Write a new commit message (note that this will be a new commit message used for sqashed commits). Every line not starting with # above, contributes to commit message.
> +* Save your changes and you are done. You will see that your commits have been squashed and you can verify this using `git log` or `git reflog`.

Remove the bullet point.
Attachment #8938703 - Flags: review?(spenades) → review-
Comment on attachment 8938703 [details]
Bug 1406383- Add documentation for squashing commits;

https://reviewboard.mozilla.org/r/209292/#review219304

Great, thank you so much for the documentation, the patience and the updates, Pradeep :D
Attachment #8938703 - Flags: review?(spenades) → review+
There are 19 opened issues, this must be marked as fixed to be able to land the patch.
Flags: needinfo?(pradeepgangwar39)
I think it was marked as review+ and I resolved everything. Do i need to add anything else to this patch?
Flags: needinfo?(pradeepgangwar39) → needinfo?(spenades)
Please take a look here: https://www.reviewboard.org/docs/manual/3.0/users/reviews/issue-tracking/#responding-to-issues 
After that, please go to https://reviewboard.mozilla.org/r/209292/diff/2#index_header, click on the ! 19 and click on "fixed" for all of them
Flags: needinfo?(pradeepgangwar39)
Thanks! I marked them fixed.
Flags: needinfo?(spenades)
Flags: needinfo?(pradeepgangwar39)
Pushed by ccoroiu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/128fc731770c
Add documentation for squashing commits; r=sole DONTBUILD
https://hg.mozilla.org/mozilla-central/rev/128fc731770c
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Product: Firefox → DevTools
Assignee: nobody → pradeepgangwar39
You need to log in before you can comment on or make changes to this bug.