Re: [ovirt-devel] patch granularity, extracting patches

Hi, I think I have found a rule which applies to lot of CRs. If you submit set of small patches, you will be asked to squash them, and if you submit 'big bang' patch, you will be asked to split it. Sometimes, and that's the hardest situation, you are asked to do both :D I think you should lead with what is most effective for you to work with. And that ideally should be (for your own good) small commits with one (or some small number) logical change (rename this, add this) in local feature branch. That way you can easily remove some change if it's not needed in the end, you have patches ready for 'fine granularity reviewer', and if you need to provide big bang patch of 'bulk reviewer', you can create one in seconds. And potential 're-split' is easy too, you just squash them again differently. Squashing is easy, splitting is not. _warning_mine_opinion_: About patch size in general. It's like method / class names. If it's called 'perform' and have 100 LOC, it's most probably bad. Thus if your patch description does not list all the changes and/or its description is excessively long: "I did X and Y, then Z, while W, unless Q", then your patch is big, and kinda defies purpose of VCS. Next time you should write smaller one, and squash it to bulk patch only if reviewer insists. Also patch "rename methodA->methodB" takes no time to review(eyes are really good to see same patterns), with small probablility of error, and when we all use IDE for refactorings, there's no need to review it at all. Five renames and method extractions in one patch, garnished with added (even trivial) new functionality is significantly harder to review, and reviewer really needs to pay attention to every line change. M. On Thu, Jun 29, 2017 at 6:16 PM, Greg Sheremeta <gshereme(a)redhat.com&gt; wrote: