I won't make big pull requests (repeat 500 times)

Lately we are getting some pull requests through into ManageIQ, and some people has come to ask me some questions...

What size should a pull request have?

Possible answers:

1. Complete functionality (i.e. that big awesome pull request that creates a new chargeback, fixed the world and praises God at the same time)
2. Independent functionality (i.e. that big pull request that needs others)
3. Something that works (i.e. complete feature but only part of a use case)
4. A small pull request (i.e. nothing works here but it is fine)

Choose wisely.....






And the answer is:

"I - don't - know".  It would depend on the community you are working with. But I will say what have been working for us for ManageIQ, as we have tried all of them.

Option 1 is just unfeasible. We are doing Scrum, so we do have functionality that is working every 3 weeks or so, it wouldn't make sense to wait more, and who wants to wait 3, 4 or 18 months to merge it?

Option 2 is fine for you, you code it, you try it, you merge it... but you need to think of the full process.
Somebody needs to go line by line, review the code, see what is does, even test it, and then say "yes" before the merge. We have a pull-request like this here.
Does it work?... I wouldn't say yes, it is a bad idea (just see the number of commits after we thought it was finished). Understanding 1300 lines of code is not easy.

We managed to get the 7th Cavalry Regiment helping, and thanks God we did, as we were missing too many things to get it merged (more about this in a later post), but to make it short: 1300 lines of code is not a fun thing to review.
So we have made some fine people lose time, first reviewing and then re-coding on our behalf. A better option: continue reading.

Option 3 is fine. In fact it works, just have a look at (this, or this). There is only one small problem with it. Everything is related, so a change in one file will make everything to be reviewed again (did we talk about wasting valuable time?). So great for small features, what about those big ugly ones?

Option 4 is the one. Git has a nice feature: you can create branches that are related to other branches, so the process is slightly different:

1. Create a branch in git for an specific part of the feature you want to code
2. Create a pull-request and mark it as work in progress (wip) with a tag and perhaps call it [WIP] Whatever - You can change the title afterwards.
3. Mark all other related pull-request as dependent on the first one.
4. Take out the wip tag and name when ready
5. Wait until the review is done and your pull request is approved, but not merged, as it depends on others
6. Continue working on the other until they are ready
7. Rebase... rebase... rebase... rebase (yes, somebody else will be working on those files)
8. See them merged all together

Did I tell you that 4 should be following 3? Do it, sometimes a use case can be split in different features, and you can merge them one per time until you finish.

Nice? Wait until we talk about testing


https://www.flickr.com/photos/nostri-imago/4999513205