Git History
Keeping the code history squeaky clean is critically important!
Note
Its a common practice to review the git log history by using git-log and git-blame. This history is important and vital to understanding the evolution of the code and how to effectively modify it without repeating historical mistakes.
This is not as academic a point as it might seem. Almost every day we dig through code while troubleshooting, and one of the most useful things is finding the ticket related to a line of code.
We use git blame for this, and if the last time a line of interest was modified (or lines around it) wasn’t functional, we have to find the parent of that commit and re-run blame. We have to repeat this process for each non-functional change. It’s damn annoying.
It also has the affect of making a section of code appear to be modified by many authors over a long period of time, which might indicate a troublesome area of code, even though the code is exactly the same functionally as the first and only time it was really written.
The following recommendations are made in order to maximize the clarity of the history and blame records:
Reviewing Code
Code reviews are an essential part of software engineering. We normally review code in Pull Requests or a “PR”.
Reviewing new code: Always Check for Style Errors
Getting style correct initially avoids cleanup or stylistic annoyances later. You don’t want to fix formatting/style errors later because they muddy the log and blame history.
Reviewing old Code: Never Make Style Corrections
Minimizing stylistic changes for defects allows the diffs to isolate defect changes alone. This allows the debug team to easily identify significant changes that breaks functionality, and fix it.
If stylistic corrections are to be made, they should be done for:
Code development
Major cleanup
Changes in the PR code itself
Don’t make unrelated style fixes for defects.
Avoid Many Small Commits for Defects
This doesn’t mean that you should have super-huge 10000 line commits. It does mean that:
Defects (and resulting PRs) should be minimal in scope to limit changes.
Small commits that are related should be put into the same commit. See above sections on how to Avoiding Many Small Commits and Squashing Multiple Commits .
Rebasing against Master (or any other branch)
Its often desirable for you, the author, to rebase your branch against the originating repo, because only you can prevent merge conflicts (forest fires). This is important so that the GitSmeg UI merges are clean and safe. When the number of commits backs up, and someone is left to do these risky merges and rebases, we can easily end up with hamburger (see DeadPool for the gory details).
Instructions:
Call your feature branch feature/ACME-1234
From your feature branch, pull a clean master:
git checkout master git pull get checkout feature/ACME-1234 # go back to your branch now.
From your feature branch, do a rebase with the -i flag:
git rebase -i master
Now if the rebase is clean, you are done. Stop.
If the rebase didn’t go well, you have to resolve conflicts: For each conflict file:
Edit the file, fix the conflict as usual.
Add the file back to the commit pool:
git add path/to/file
Once all your conflicts are resolved as above, continue the rebase:
git rebase --continue
Note: you may have to go back to the previous step and resolve more issues if this fails.
If you continue to have problems call the git police (ask for help in Slack).
Now that your rebase is clean, push your feature back upstream:
git push -f
Now your PR should be clean and ready to merge. Go check it in GitSmeg.
Push the Develop Branch onto the old Feature that is Stale
Warning
This flow can be dangerous. Use with caution!
You have created a branch (forgotten) that has been left behind and wish upgrade it with all the new changes that have been made with other feature enhancements. You don’t have anything to save in it. Use these commands (with caution) to merge develop back onto feature/forgotten:
[bash]: git checkout feature/forgotten
[bash]: git push . develop:feature/forgotten
[bash]: get checkout feature/forgotten
[bash]: git commit -a
[bash]: git push
Push a new Feature up to Origin for storage:
Sometimes you want a feature to be stored on Git cloud provider. You can push it up to GitSmeg like this:
[bash]: git push -u origin feature/new
Heavy Feature Workflow IMPORTANT: PLEASE READ
During heavy workflows on a project, we expect multiple teams to concurrently work on multiple features that get merged into develop. This is common. This heavy workflow can be managed by the following workflow:
Create your feature/area51
Work/commit/push to feature/area51 branch
Another team does works on feature/Atlantis (don’t be jealous!)
Another teams merges feature/Atlantis into develop
Now you need to rebase those changes into feature/area51 as follows:
git checkout develop && git pull # Get feature/Atlantis changes git checkout feature/area51 git rebase develop
If you have conflicts see merge_conflicts below, else continue
Now continue work on feature/area51
Repeat the rebases as needed when team Atlantis updates develop
Once finished, create your final Pull Request
Once merged, delete the feature branch.
Rebasing a Feature on Develop
Warning
If your team is still working on a feature and you notice that develop has been updated, you should try to rebase those changes into your feature. This will avoid conflicts later when you merge back into develop.
You may get these messages
Note
Branches ‘develop’ and ‘origin/develop’ have diverged. Fatal: And branch ‘develop’ may be fast-forwarded.
Someone has added to develop during your work on feature/area51. This is common in a multi-user environment. You will have to merge the two together. To solve this, you need to:
Sync local develop with origin: checkout develop, pull from origin to develop:
git checkout develop && git pull origin
Rebase your feature on develop. You may have conflicts here if you’re unlucky:
git checkout feature/area51; git rebase develop
Check that nothing is broken:
git status git push # Push feature/area51 up to origin
If there are conflicts you have to fix here. See merge_conflicts
Merge Conflicts: Fixing a Rebase
If you do have conflicts with your merge you can take a simple approach to fixing them:
Rebase against develop:
[joe@acme]: git rebase develop First, rewinding head to replay your work on top of it... Applying: Make Tenant rels concrete Using index info to reconstruct a base tree... M Bricks/acme/Anvil/__init__.py Falling back to patching base and 3-way merge... Auto-merging Bricks/acme/Anvil/__init__.py Falling back to patching base and 3-way merge... Auto-merging Bricks/acme/Anvil/__init__.py CONFLICT (content): Merge conflict in Bricks/acme/Anvil/__init__.py Failed to merge in the changes. Patch failed at 0001 Make Tenant rels concrete The copy of the patch that failed is found in: /data/Bricks.acme.Anvil/.git/rebase-apply/patch When you have resolved this problem, run "git rebase --continue". If you prefer to skip this patch, run "git rebase --skip" instead. To check out the original branch and stop rebasing, run "git rebase --abort".
Edit the problem file and fix:
[joe@acme]: vi __init__.py ...fix stuff here... [joe@acme]: git status rebase in progress; onto 34ae002 You are currently rebasing branch 'feature/ACME-17143_installWarnings' on '34ae002'. (fix conflicts and then run "git rebase --continue") (use "git rebase --skip" to skip this patch) (use "git rebase --abort" to check out the original branch) Unmerged paths: (use "git reset HEAD <file>..." to unstage) (use "git add <file>..." to mark resolution) both modified: __init__.py
Add this file back into to index:
[joe@acme]: git add __init__.py
Continue:
[joe@acme]: git rebase --continue Applying: Make Tenant rels concrete Applying: fix context relations Using index info to reconstruct a base tree... M Bricks/acme/Anvil/Tenant.py M Bricks/acme/Anvil/__init__.py Falling back to patching base and 3-way merge... Auto-merging Bricks/acme/Anvil/__init__.py CONFLICT (content): Merge conflict in Bricks/acme/Anvil/__init__.py CONFLICT (modify/delete): Bricks/acme/Anvil/Tenant.py deleted in fix context relations and modified in HEAD. Version HEAD of Bricks/acme/Anvil/Tenant. py left in tree. Failed to merge in the changes. Patch failed at 0002 fix context relations The copy of the patch that failed is found in: /data/Bricks.acme.Anvil/.git/rebase-apply/patch When you have resolved this problem, run "git rebase --continue". If you prefer to skip this patch, run "git rebase --skip" instead. To check out the original branch and stop rebasing, run "git rebase --abort"
Repeat: You may have to edit/re-edit a file, re-add, and continue as before:
[joe@acme]: vi __init__,py [joe@acme]: git add __init__.py [joe@acme]: git rebase --continue Bricks/acme/Anvil/Tenant.py: needs merge You must edit all merge conflicts and then mark them as resolved using git add
Delete what is required. You deleted a file but it is confused by this:
[joe@acme]: git rm Tenant.py Bricks/acme/Anvil/Tenant.py: needs merge rm 'Bricks/acme/Anvil/Tenant.py' [joe@acme]: git rebase --continue Applying: fix context relations
If at this point the merge is good, but it asks you to pull, don’t pull! You really want to push your changes:
[joe@acme]: git status On branch feature/ACME-17143_installWarnings Your branch and 'origin/feature/ACME-17143_installWarnings' have diverged, and have 14 and 2 different commits each, respectively. (use "git pull" to merge the remote branch into yours) nothing to commit, working directory clean [joe@acme]: git push To git@github.com:acme/Bricks.acme.Anvil.git ! [rejected] feature/ACME-17143_installWarnings -> feature/ACME-17143_installWarnings (non-fast-forward) error: failed to push some refs to 'git@github.com:acme/Bricks.acme.Anvil.git' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Integrate the remote changes (e.g. hint: 'git pull ...') before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details. [joe@acme]: git push --force Counting objects: 12, done. Delta compression using up to 8 threads. Compressing objects: 100% (12/12), done. Writing objects: 100% (12/12), 1.22 KiB | 0 bytes/s, done. Total 12 (delta 6), reused 0 (delta 0) To git@github.com:acme/Bricks.acme.Anvil.git + 2bfc0a6...f7ddee9 feature/ACME-17143_installWarnings -> feature/ACME-17143_installWarnings (forced update)If you see a clean status, its probably good. Make sure to test:
[joe@acme]: git status On branch feature/ACME-17143_installWarnings Your branch is up-to-date with 'origin/feature/ACME-17143_installWarnings'. nothing to commit, working directory clean