LGTM is what people say when they aren’t actually reviewing your PR. There can be many causes for this, but the most common is the PR is too freaking big.
But, but, but… it’s all related! - You
Wrong. You’re a tool.
If I had more time, I would have written a shorter [PR]. - Blaise Freaking Pascal
You can make small, independent, PRs, even with a shitty, entangled codebase. It doesn’t require perfect knowledge and it doesn’t require any special talents at anticipating what needs to be done before you start.
It does require:
- some Git skills
- a little extra time
- Big PRs lead to LGTM which undermine the review process.
- You’re a craftsman who leads by example.
Bang out the code however you see fit.
This is where hacks create PRs, but you’re better than that. At this point, your branch is just a rough draft. Don’t ever submit your rough draft.
Step 2 - reset
At this point, create another branch if you’re scared,
git reset your branch
to whatever the destination branch is. It’s probably
develop. This takes all
the precious work you did and undoes it, moving it all to the working directory.
Now that you’ve finished solving the problem, you know all the different parts needed to get the job done. You should be able to see how they work together and figure out a way to fold them in incrementally.
- add a service to return a list of foos
- add Redux stuff to manage foos (selectors, action creators, etc.)
- connect state to containers, pass foos down to components
- add a saga to dispatch the action to get the foos
Each of these is certainly related, but you can divvy them up. Granted step 1 doesn’t really do much from the user’s perspective, but that doesn’t matter. You can add it to the app. It’s infrastructure. Even if nothing is using it, it’s there and it’s solid.
Step 3 - patch
Now that you have a plan for how to introduce the elements, create new commits. You can select parts of files to stage instead of adding the whole thing with patching.
git add blah -p
Build your commits one at a time. The goal is for each commit to contain a complete “mini-feature.” And the app should work at each step. The presense of the “foo service” above doesn’t break anything. If we started from step 4, adding stuff that depends on the “foo service,” it would break.
Step 4 - rebase
Now that you’ve jacked your history, rebase on your target and push (use the force). Create your PR and know that you’re cool.
But it’s still too big!!!! - She
Yeah, as a whole diff, it’s the same amount of change, still hard to review. Use your noggin and tools to have people review each commit. You can do that in Bitbucket and Github pretty intuitively. If you’re a real badass, though, and your PR process isn’t totally crap, you can submit 4 separate tiny PRs now, since the app works each step of the way.
You’re cool. Have a pickle.