7
\$\begingroup\$

I often find myself doing rebase manually in an interactive form in a way as follows. Say I want to squash two last commit to third commit.

First I execute:

$ git rebase -i HEAD~3

Now editor pops up, I mark two last commits as fixup. I close the editor and then rebasing is done (if there are no errors). I'd like to automate this by executing a script with a parameter that gives a number of commits, i.e. when 2 is given as parameter, last two commits are fixup to third commit. Please see an example:

some other commits...

add nice feature

fix 1

fix 2

In that scenario I'd like add nice feature to incorporate changes from fix 1 and fix 2, and I like to fix 1 and fix 2 not be present at all.

I wrote a bash script that does what I want. It works seems to work fine.

UPDATE

In this review, I'm most interested in knowing whether my approach to git is a proper one, especially if it doesn't break anything during rebasing.

fixup() {
  local no_of_commits="$1"
  if [ -z "$no_of_commits" ]; then
    echo "You must provide a number of commits to fixup!"
    return 1
  elif ! [[ $1 =~ ^[0-9]+$ ]]; then
    echo "$no_of_commits is not a number!"
    return 2
  fi

  #git stash save

  git reset --soft "HEAD~$(no_of_commits)" &&
    git add --all &&
    git commit --fixup "$(git rev-parse HEAD)" &&
    GIT_SEQUENCE_EDITOR=true git rebase --interactive --autosquash --no-fork-point "$(git rev-parse HEAD~2)" &&
    echo "Rebased $(no_of_commits) succesfully!"

  #git stash pop
}   
\$\endgroup\$
  • 1
    \$\begingroup\$ I'd suggest using [[ consistently instead of [. It will help you avoid surprises like variables ending up empty. In your case quoting the variable takes care of it, but [[ is still a good habit to get into. \$\endgroup\$ – chicks Jan 14 at 14:02
2
\$\begingroup\$

Syntax errors

The posted code has some syntax errors:

  git reset --soft "HEAD~$(no_of_commits)" &&
    git add --all &&
    git commit --fixup "$(git rev-parse HEAD)" &&
    GIT_SEQUENCE_EDITOR=true git rebase --interactive --autosquash --no-fork-point "$(git rev-parse HEAD~2)" &&
    echo "Rebased $(no_of_commits) succesfully!"

That is, all the $(no_of_commits) must really be $no_of_commits or ${no_of_commits} if you like.

Use git commit --amend

git commit --fixup is useful when you will have a bunch of fixup commits in the midst of other commits. To squash together the last N commits, is a special case, and can be done simpler using git reset ... && git commit --amend:

  git reset --soft "HEAD~$no_of_commits" &&
    git commit --amend -C HEAD
    echo "Rebased $no_of_commits successfully!"

I also dropped the git add --all in the middle, because it's not necessary for the purpose you described, in fact it may have unintended effects. That is, any uncommitted changes will get added. If I want to fixup the last N commits, I would want "just that", and nothing else. If I wanted the uncommitted changes included, I would commit them.

Use more functions

I would extract the conditional that checks if $no_of_commits is a number to its own function. Then you could easily copy-paste and reuse in other scripts.

Also, the elif should use $no_of_commits instead of $1.

About rebasing...

In this review, I'm most interested in knowing whether my approach to git is a proper one, especially if it doesn't break anything during rebasing.

Collapsing the last N commits isn't really rebasing, because no commits are applied on top of some other commit, it's really just amending a commit.

As mentioned in the previous section, I think the git add --all operation is a mistake, which I would consider a defect of the otherwise nice functionality.

\$\endgroup\$
2
\$\begingroup\$

The error messages should go to standard error, rather than standard output:

  if [ -z "$no_of_commits" ]; then
    echo "You must provide a number of commits to fixup!" >&2
    return 1                                              ### HERE

I don't think there's a good case for returning distinct error codes for missing argument and invalid argument. Are you ever going to make use of the different status values?

\$\endgroup\$
  • \$\begingroup\$ No, I'm not. One return value will suffice. \$\endgroup\$ – menteith Jan 14 at 14:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.