A great way to make your pull requests easier to review is to reduce each commit to a particular purpose/functional change. In practice this often means not combining multiple feature additions in a single commit, or not including whitespace changes in a commit that makes some code change.
git add is just not great for anything but very trivial changes, so I use GitX (the active fork) when I’m on OSX.
Getting to the UI is a quick
gitx in terminal and command+2 to switch to the Stage view. Here you can move changes in/out of staging via drag/drop files or by clicking on individual lines in diff views. This lets you run wild and loose during development then, with a scalpel, carefully craft your changes into a logical set of cleaner commits. E.g. if you’ve altering code, unstage all your unrelated whitespace changes and put them in the last commit, or just revert them or
git stash to move them to another branch.
There’s no perfect way to develop software and use source control because projects, teams, and work environments can vary so much; what works for a small office of employees might not for a loose group of part-time contributors spread across many timezones, as many open source projects are.
Jade Rubick is not a fan of long-running feature branches in git and—if I’m reading this right—argues for merging into master frequently, not waiting for an entire feature to be implemented. This is supposed to force the team to be aware of all code changes occurring.
While lack of communication about features in development can certainly be problematic, I think this is a sledgehammer of a solution. Taking this approach to its extreme, it might make sense to have all developers work huddled together so they can say what they’re working on in real-time, or all work on one workstation. My point is that using a workflow with high costs to address a communication deficit is not so great an idea.
My big fear of this workflow is that it eases the flow of incorrect/unwise code into production. Who reviews this code? What if it takes the whole codebase in a bad direction, but no one at the moment has time to realize that? I think the benefits of feature branches/pull requests are just huge:
- A branch frees the developer to experiment big and take chances without forcing the rest of the team down their path. The value in some big ideas will not be apparent looking at them piecemeal. Some of this work will lead to great things, some will be tossed away, all of it will be good learning.
- Likewise, the PR process can catch incorrect/unwise solutions before they’re merged into the product. This is huge. Some ideas sound great but you only realize 80% into the work that they’re unwise. If that work is sitting in a PR, you just close it and it can live on as a reminder to future devs who get the same idea. If not, you now have the job of shoehorning that code out. On the codebases I work on so many features have been improved/overhauled/abandoned by the review/feedback loop that it seems absolutely crazy to bypass this process. In an async distributed team, I think the PR is basically the perfect code review tool.
- PRs provide a great historical and educational record of what changes are involved in providing a certain feature, what all files are involved, etc. I’ve found that reading pull requests and merge diffs to be just as illustrative as reading source code. If a feature required changes in dozens of files over three weeks, how will I ever piece together the 6 commits out of 100 that were important?
- Feature branches make it a lot easier to revert a feature or apply it to another branch. For life on the edge I’ve built upon versions of frameworks with experimental branches merged in. If I regret this I can always generate a revert commit to sync back up with a stable branch.
All workflows have costs/benefits, I just think that the benefits of not merging feature branches until they’re really ready are huge compared with the costs Jade described.
My hunch is there must be better ways to keep a team aware of other work being done on feature branches. E.g. Make pull requests as soon as the feature branch is created and push to it as you work. That way team members can set aside time to check in on pull requests in progress and provide feedback.
I agree with Jade that feature flags can be a great idea, but that’s mostly orthogonal to source control workflow.
I wanted to add a little feature to the Delicious Chrome extension, but couldn’t find any reference to how to edit/clone a 3rd-party extension. It turns out—at least for making minor mods—it works how you’d think:
- Change some extension files.
- Restart Chrome.
Caveat: Undoubtedly an update of the extension would wipe out these changes, and following this advice might break the extension (or Chrome!).
Adding a link to the Delicious Bookmarks Extension
On XP, the extension files were in
C:\Documents and Settings\<user>\Local Settings\Application Data\Google\Chrome\User Data\Default\Extensions\lnejbeiilmbliffhdepeobjemekgdnok\0.998_0.
To track my work, I did
git init; git add . then repeated these steps:
- Change file(s)
- Restart Chrome
- Test. If OK,
git commit -m 'desc of changes'
When done I’d added a simple link to the user’s bookmarks (screenshot). I wanted to make this easy for the Delicious team to incorporate these changes, so I generated this patch by diff-ing from the oldest commit:
git diff $(git rev-list HEAD | tail -n 1) > ~/del-chrome.patch