swcarpentry / shell-novice

The Unix Shell
http://swcarpentry.github.io/shell-novice/
Other
372 stars 969 forks source link

Cleanup commit-message #9

Closed rgaiacs closed 9 years ago

rgaiacs commented 9 years ago
rgaiacs commented 9 years ago

@wking can you check if this are the commit that need a cleanup at the commit-message? I can do the cleanup after get a green light.

wking commented 9 years ago

On Fri, Oct 31, 2014 at 06:23:16PM -0700, Raniere Silva wrote:

  • [ ] 69fe2c3303fea8db46242229918f5e848e3daaee
  • [ ] 75ebce5fd6e60a7e651d8b38cbba03ffe93db023
  • [ ] 3671a8ac31166e8b973f9d46e208164aca500789
  • [ ] 79b05edae02e23bf8499fa76b89c02456131b372

Just so we can find them again after rebasing, I'll copy down some of those commit summaries for posterity:

Also long and worth adjusting (e.g. it's missing the blank line between the summary and the body):

Don't worry about a PR for this. Just let me know when you've got them fixed, and point me at a branch in your public repo.

I'd usually handle this sort of thing with ‘rebase -i’, but that doesn't play well with a mergy history. It might be easiest to do something like:

$ git checkout -b wip $LONG_COMMIT $ git commit --amend … fix the message … $ git checkout master $ git rebase -p --onto wip $THE_COMMIT_AFTER_THE_LONG_COMMIT $ git branch -d

You'd want to start with the newest long commit and work your way back to avoid having the hashes change under you, although you could always look up the new hash by searching for the summary text in the logs.

rgaiacs commented 9 years ago

@wking Done at https://github.com/r-gaia-cs/swc-modular-shell/tree/cleanup.

wking commented 9 years ago

On Sat, Nov 01, 2014 at 05:23:46AM -0700, Raniere Silva wrote:

@wking Done at https://github.com/r-gaia-cs/swc-modular-shell/tree/cleanup.

Hmm, looks like you dropped the commit after the long message:

$ git diff master..r-gaia-cs/cleanup diff --git a/00-intro.md b/00-intro.md index 8da32a0..502b1bf 100644 --- a/00-intro.md +++ b/00-intro.md @@ -117,7 +117,7 @@ Commands are terse (often only a couple of characters long), their names are frequently cryptic, and their output is lines of text rather than something visual like a graph. On the other hand, -the shell allows us to combine existing tools in powerful ways with only a few keystrokes. +the shell allows us to combine existing tools in powerful ways with only a few keystrokes and to set up pipelines to handle large volumes of data automatically. In addition, the command line is often the easiest way to interact with remote machines. … $ git log --oneline master | grep -A1 'Adding links to reference' 271097f Adding links to reference material on appropriate pages 39df817 Fixes #546 by using level-3 headings in all Markdown source files for subsections. We continue to use level-4 headings for objectives, key points, callout boxes, and challenges. $ git log --oneline r-gaia-cs/cleanup | grep -A1 'Adding links to reference' $ echo "$?" 1

I guess it should have been:

$ git rebase -p --onto wip $LONG_COMMIT

You could patch those commits back into your cleaned-up history, but it might be easier to just:

$ git reset --hard origin/master

and start over (grepping through ‘git log -p r-gaia-cs/cleanup’ to grab your new commit messages for copy-pasting and finally wrapping up with a ‘git push -f r-gaia-cs cleanup’).

rgaiacs commented 9 years ago

@wking Can you check again? Hope this time is OK.

wking commented 9 years ago

On Sun, Nov 02, 2014 at 01:00:52PM -0800, Raniere Silva wrote:

@wking Can you check again? Hope this time is OK.

Hmm. This time the output matches :), but it looks like we're flattening some history:

$ diff -u <(git log --graph --topo-order --pretty=format:%s origin/master) <(git log --graph --topo-order --pretty=format:%s r-gaia-cs/cleanup) … @@ -55,12 +52,11 @@

The “Removing explicit 'level' keys from Markdown files” commit seems to have been shifted into the first-parent branch, and now it lands before “Switching to explicit

 blocks…”.  It also looks like
we've dropped the long “Getting rid of old cheat sheets…” commit, but
that's fine, because the only changes we see from that commit were
duplicated in the “Removing explicit 'level' keys from Markdown files”
commit.  If this flattening was intentional, it would be nice to have
some notes about this stuff.  Otherwise I'll try and find time to look
through the revised history to check that the changes make sense.

rgaiacs commented 9 years ago

If this flattening was intentional, it would be nice to have some notes about this stuff.

It isn't.

Otherwise I'll try and find time to look through the revised history to check that the changes make sense.

Sorry for the trouble.

wking commented 9 years ago

On Mon, Nov 03, 2014 at 09:03:11AM -0800, Raniere Silva wrote:

Otherwise I'll try and find time to look through the revised history to check that the changes make sense.

Sorry for the trouble.

No trouble, it's good to know what commits needed fixing, and to know where the history was tricky. However, it's hard to restore flattened history, so I ended up just starting over from master. Start a new branch:

$ git checkout -b commit-message-cleanup origin/master

Look around the the newest commit @r-gaia-cs found for some context:

$ git log --graph --topo-order --oneline --decorate | grep -2 'using level-3 headings in all Markdown' |/|

Fix that commit:

$ git checkout -b wip 39df817 $ git commit --amend …add a blank line between the summary and body… $ git checkout commit-message-cleanup $ git rebase -p --onto wip 39df817

Check our work:

$ git log --graph --topo-order --oneline --decorate | grep -2 'using level-3 headings in all Markdown' |/|

So we have the same graph, with a better commit summary :). Kill the wip branch, since we don't need the marker anymore:

$ git branch -D wip

Moving on to the next oldest commit, we'll get some context:

$ git log --graph --topo-order --oneline --decorate | grep -4 'In the novice shell lesson 03-pipefilter'

Fix that commit:

$ git checkout -b wip 69fe2c3 $ git commit --amend …add two newlines after the summary sentence, and wrap the body… $ git checkout commit-message-cleanup $ git rebase -p --onto wip 69fe2c3

Check our work:

$ git log --graph --topo-order --oneline --decorate | grep -4 'In the novice shell lesson 03-pipefilter'

So we have the same graph, with a better commit summary :). Kill the wip branch, since we don't need the marker anymore:

$ git branch -D wip

Moving on to the next oldest commit, we'll get some context:

$ git log --graph --topo-order --oneline --decorate | grep -4 'Getting rid of old cheat sheets'

The summary for 75ebce5 suspiciously contains “Removing explicit 'level' keys from Markdown files”, which is the whole summary for 114fa8d. Lets see what's actuall in 75ebce5:

$ git show 75ebce5 | grep '^[-+]' --- a/00-intro.md +++ b/00-intro.md -level: novice --- a/01-filedir.md +++ b/01-filedir.md -level: novice …

Maybe we don't need this commit at all. Comparing 114fa8d with the subsequent merge in 60d1cf5:

$ git cat-file -p 114fa8d | grep tree tree b41ba26aa06eeaadb6422034261d526f1b68590b $ git cat-file -p 60d1cf5 | grep tree tree b41ba26aa06eeaadb6422034261d526f1b68590b

Lets just drop the long gutted commit (75ebce5) and the useless merge (60d1cf5):

$ git rebase -p --onto 114fa8d 60d1cf5

Check our work:

$ git log --graph --topo-order --oneline --decorate | grep -2 'keys from Markdown files' |/

So we've completely dropped the branch that 75ebce5 was on, along with its subsequent merge :). Moving on to the final wordy commit, we'll get some context:

$ git log --graph --topo-order --oneline --decorate | grep -2 'Corrects runtime calculation for goostat and goodiff' |/

Fix that commit:

$ git checkout -b wip 79b05ed $ git commit --amend …add two newlines after the summary sentence, and wrap the body… $ git checkout commit-message-cleanup $ git rebase -p --onto wip 79b05ed

Check our work:

$ git log --graph --topo-order --oneline --decorate | grep -2 'Corrects runtime calculation for goostat and goodiff' |/

So we have the same graph, with a better commit summary :). Kill the wip branch, since we don't need the marker anymore:

$ git branch -D wip

Compare with our original tree:

$ git cat-file -p origin/master | grep tree tree d4903d6a5dae671dd3e07cf8d853966b131b475e $ git cat-file -p HEAD | grep tree tree d4903d6a5dae671dd3e07cf8d853966b131b475e

so we haven't lost any content. What about our graph?

$ diff -u <(git log --graph --topo-order --pretty=format:%s origin/master) <(git log --graph --topo-order --pretty=format:%s HEAD) --- /dev/fd/63 2014-11-05 21:14:12.639503264 -0800 +++ /dev/fd/62 2014-11-05 21:14:12.639503264 -0800 @@ -27,10 +27,10 @@ | |/
|/|

Looks great :) (I've truncated the long commit messages by hand in that paste, because they make it hard to read the diff). Adjust our local master to point at this commit, and drop the temporary commit-message-cleanup branch name:

$ git checkout master $ git reset --hard commit-message-cleanup $ git branch -D commit-message-cleanup

Push to the public repo, clobbering the old master:

$ git push -f origin master

wking commented 9 years ago

Ah, I missed swcarpentry/bc@b94793c (1. Making filenames in find example match…, 2014-02-10). Find some context:

$ git log --graph --topo-order --oneline --decorate | grep -2 'Making filenames in'

Fix the commit:

$ git checkout -b wip 7a90443 $ git commit --amend …I wrote a new summary line here… $ git checkout master

Lets show the viscinity before the rebase this time, just so everyone's clear on the --onto syntax:

$ git log --graph --topo-order --oneline --decorate wip 52fcfd4 | head -n 6

So the rebase is taking everything in master that's after 7a90443 (starting with 60184e3) and grafting it onto wip (ca90754). That drops 7a90443, and replaces it with wip (ca90754) which has the shorter summary:

$ git rebase -p --onto wip 7a90443

Check the results:

$ git log --graph --topo-order --oneline --decorate | grep -2 'adjust filesnnames'

Looks good. Kill the wip branch:

$ git branch -D wip

And publish (clobbering the old master):

$ git push -f origin master …

wking commented 9 years ago

In the previous two comments I explained how I cleaned up the commit messages. It's nice to ask folks to acknowledge changes like that though, to avoid putting words into their mouth. I was mostly shifting whitespace around, but still, the reworded messages were:

Could you each just check the changed commit messages (the commit hashes should be clickable in the GitHub web UI) and confirm that they're acceptable?

wking commented 9 years ago

On Wed, Nov 05, 2014 at 09:46:49PM -0800, W. Trevor King wrote:

  • swcarpentry/bc@a6fd62d (Fixes #546 by using level-3 headings in all Markdown…, 2014-06-18) → 194ef9f. @gvwilson

Heh, although the GitHub UI doesn't actually show the difference here (it does show differences for the other altered commit messages). Here's the Git UI for this particular change:

$ git log -1 --oneline a6fd62d a6fd62d Fixes #546 by using level-3 headings in all Markdown source files for subsections. We continue to use level-4 headings for objectives, key points, callout boxes, and challenges. $ git log -1 --oneline 194ef9f 194ef9f Fixes #546 by using level-3 headings in all Markdown source files for subsections. $ diff -u <(git log -1 a6fd62d) <(git log -1 194ef9f) --- /dev/fd/63 2014-11-05 21:51:00.729560521 -0800 +++ /dev/fd/62 2014-11-05 21:51:00.729560521 -0800 @@ -1,6 +1,7 @@ -commit a6fd62d2baffad9d41f7391c1ddfd4d37ad3048f +commit 194ef9fa9573be7360f1b1af069541d289e88518 Author: Greg Wilson gvwilson@third-bit.com Date: Wed Jun 18 08:07:43 2014 -0400

   Fixes #546 by using level-3 headings in all Markdown source files for subsections.
wking commented 9 years ago

We're now live with additional post-bc history, so the window for feedback from @mestato and @BernhardKonrad has closed.

BernhardKonrad commented 9 years ago

Sorry, I didn't realize someone was waiting for my input. It all looks good!