Git workflow to audit commits without rewriting history

Question

I am working with a developer that is new to git, and I would like to setup a git workflow that would let me auditing the commits made by this developer (and possibly reject them) without forcing him to rebase his work (which is error prone for new users).

Here is the scenario :

• the master branch contains only audited code
• devel branch is created by forking master branch
• a developer is working on the devel branch and push his code for me to audit
• I detect some issues with his code, so I ask him to make more commits on devel branch to fix the issue
• Once I'm happy with the code, I cherry-pick (or merge with squash) the developers commits on the master branch, add my own comments and mark the commit as authored by the developer
• The developer merge back the master branch into his devel branch

Here is a visual illustration of this scenario :

After this sequence, how can I be 100% sure that mistakes made by the developer during the "make mistakes / redo" sequence will not show up again when the developer will push his devel branch again after some other commits ?

The best solution would be to ask the developer to rebase his devel branch against the master one, but this is a hard task to new git users.

If this workflow is wrong in any way, please suggest me another where I would be able to audit the commits before merging them into the master branch.

EDIT

A friend suggested me a direction that looks promising : the --fixup option when doing commit.

--fixup=<commit>

Construct a commit message for use with rebase --autosquash. The commit message will be the subject line from the specified commit with a prefix of "fixup! ". See git-rebase for details.

This option could be used by the developer to properly mark his second and third commits as a fix for his first one. A good solution to explore...

Answer 1

Remark 1: By trying to turn the separated commits on devel into a single commit on master, you are rewriting history. I see the point in trying to keep a clean, linear history on master, however a less linear history with merges would be more easily integrated with git basic commands.

Remark 2: If your workflow explicitly includes members which are not familiar with the basic tools you use to share code, you should live with the consequences. At some point, I think the developer should learn git.

Here are two suggestions :

1. Drop the linear history on master, and "validate" the changes by running, from master :

   git merge --no-ff devel
   

   This is the most straightforward way to inform git that the modifications on devel are taken into account on master.

2. Do the merge with devel yourself, and have your developer pull the modifications from origin/devel.
   If the single commit on master really only consists in squashing the commits on devel, this shouldn't trigger any conflicts -- the merge commit master->devel should introduce 0 modifications.
   If you have actually modified the squashed commit, and the developer can have local modifications of his own, you have to face the possibility of triggering conflicts on his side anyway ...

Answer 2

If you're going to just squash the dev branch instead of merging it normally, it's not a good idea to then merge that squashed commit back into the dev branch, because then you'll get a ton of confusing conflicts, because git thinks you're applying new code, when in fact it's actually the same code.

It would be better to either just do a simple merge into master, then a simple merge back into devel, or otherwise throw-away the devel branch and make a new one off of the most recent commit of master. As long as the code that you merge into master is "good", according to you, then you probably won't get any regressions.

On the other hand, if you go with your original plan of merging in squashed commits back into devel, that will increase your chances of a regression occurring, because of the confusing conflicts.

Update

I deleted parts of my answer above because I actually just tested doing the squash commit on master and then immediately merging it back into devel, and it didn't cause any conflicts, so that part of my answer was incorrect.

I turned this into answer into a community wiki because it still contains a lot of information about the problem in the comments.

Answer 3

Just use gerrit. With gerrit you will have the history of your comments, commits etc. You can also abandon change. The workflow I use as a developer:

Prepare

Copy gerrit commit-message hook to your repo/.git/hooks. Make sure it's executable and doesn't contains crlf.

Make remote review for pushing, or better shell alias if you work on *nix.

Make alias for rebasing in ~/.gitconfig (about fork-point)

[alias]
        reb = !"git rebase -i $(git merge-base --fork-point origin/master HEAD)"

Workflow

while unapproved do
    git commit -a --amend
    git push review
    # reviewers comment, reject, approve etc.
    # If rejected, I change commit as required, then
end

Squashing

When pushing to gerrit there should (probably) be only one commit.

So to squash commits on your branch into one:

git reb

Editor will open. Change pick to squash for every line except first (oldest commit). Save and quit. Editor will open with commit messages. Delete everything except oldest commit message and the line with Commit-ID (it's needed for gerrit).

Rebasing

If merging your branch fails in gerrit, you can rebase it onto origin/master

git rebase origin/master

If there are many conflicts, you can avoid pain of resolving many of them just by using cherry-pick

git checkout -b topic2 origin/master
git cherry-pick topic
git branch -D topic
git branch -m topic

git review

You can also use git review to simplify some operations when working with gerrit.

Answer 4

If you make sure your other developer always creates changes from a certain branch, say develop, you can have main branch setup that always exist for example:

master, develop

If your developer only creates feature branches from the develop branch he can create commits all he wants. Then when you think it is ready you can cherry-pick/rebase and do your thing with it. The feature branch gets deleted making sure rewritten history is gone.

All your developer needs to remember would be to create a branch from the remote develop branch everytime he wants to start a new feature and not touch his local feature branch when you say you're going to merge some stuff into your develop.

Then when it is ready he deletes his myfeature branch locally and you delete it remote when you've merged/rebased/cherry-picked his good changes into your develop.

Only the good commits go into develop branch. Your developer creates new branch from develop like before.

I think some basic branching is the least your developer should know. If he cannot rebase/clean his branch/commits I think you have to agree to follow a certain workflow.

Maybe not perfect, but perhaps a possible workflow nonetheless. Hope it helps.

Answer 5

The easiest way with the least amount of fuss would just be to merge in the changes from the devel branch into master. You just need to regularly update the branch with master so that your branches don't diverge too wildly (depending on what features other people are working on).

If you want to have one single commit for the modification on master. You can rebase the developers commits into one single commit and then cherry-pick that commit onto master. This would lead to complications as you would only be able to do git push -f to update the remote devel branch and if your developer is not familiar with git would be complicated for updating their devel branch. Or you would require that they delete that branch and then use a new branch for any future work.

You will need to decide which option to go on based on whether or not you want to have all the commits from your developer showing up in the log. The easiest and least troublesome way is just to simply merge devel and master.

Answer 6

I will test the following workflow and let you know if things are going well.

Prerequisites

• developer must be able to commit from the command line
• developer should always stay on the same branch, so that he does not need to worry about which branch is currently checked out
• developer should never have to solve conflicts that impact code he does not have written
• developer will never be in a situation where he could lost local modifications to its working tree

Tools

git commit with the --fixup option :

--fixup=<commit>

Construct a commit message for use with rebase --autosquash. The commit message will be the subject line from the specified commit with a prefix of "fixup! ".

git rebase with --autosquash, --autostash and --interactive options

--interactive

Make a list of the commits which are about to be rebased. Let the user edit that list before rebasing. This mode can also be used to split commits.

--autosquash

When the commit log message begins with "squash! ..." (or "fixup! ..."), and there is a commit whose title begins with the same ..., automatically modify the todo list of rebase -i so that the commit marked for squashing comes right after the commit to be modified, and change the action of the moved commit from pick to squash (or fixup). Ignores subsequent "fixup! " or "squash! " after the first, in case you referred to an earlier fixup/squash with git commit --fixup/--squash.

This option is only valid when the --interactive option is used.

If the --autosquash option is enabled by default using the configuration variable rebase.autosquash, this option can be used to override and disable this setting.

--autostash

Automatically create a temporary stash before the operation begins, and apply it after the operation ends. This means that you can run rebase on a dirty worktree. However, use with care: the final stash application after a successful rebase might result in non-trivial conflicts.

Workflow

1. The developer fork the master branch into a devel branch. The fork point is marked with a tag (for exemple version_1.1)
2. The developer work and commit on the devel branch. When the work is done, he pushes his branch on the remote repository so that the auditor can see and validate it.
3. The auditor fetch the devel branch and look at the code. If something is not correct, he asks the developer to make some modifications.
4. For each modification done by the developer, the following command should be used to commit the correction : git commit --fixup <commit that was not OK>. Than the branch is pushed again.
5. Once the auditor is happy with the modifications done, the developer must rebase his branch in order to squash all the fixup commits into one beautiful and error-free commit (other unrelated work could have been commited meanwhile).

GIT_EDITOR=: git rebase tags/version_1.1 --interactive --autosquash --autostash.

The GIT_EDITOR=: is here to avoid to display the editor that present the commit lists due to the --interactive option. This option is needed to make the fixup commit reorder automatically.

The last step is to push the devel branch again (with --force option) so that the auditor can merge it in the master branch. The rebase base for next devel fixup could be the master branch or a new tag that the developer put on his devel branch after merging back the master branch.

Here is the resulting git tree :

Answer 7

Well, if you have one beginner developer and you are the only reviewer, i suggest this workflow:

1. the developer create a local branch from master to implement some feature (branch name: i-123)
2. the developer add some code to branch i-123
3. the developer create merge request to merge i-123 on master branch
4. the reviewer find some mistake and notifies the problem to developer
5. the developer add commits to solve problem and notifies to reviewer to check change of i-123 branch
6. the reviewer verifies the source code and accept merge request (merge i-123 on master branch as result i-123 was destroyed)
7. the developer sync local repository (as result: master branch contain change relative to i-123 and i-123 was destroyed)
8. the developer can continue with step 1

This approach allow that one developer work on one or various features at same time, and eliminate complicate logic about mantein updated develop branch. It's easy approach to the beginner developer.