Jan Stolarek
(did you intend to send two identical links?) No :-) The branches should be js-hoopl-cleanup-v1 and js-hoopl-cleanup-v2
Yes, the end result would be the same - I'm merely asking what would be preferred by GHC devs (i.e., I don't know how fine grained patches to GHC usually are). I don't think this should be visible in your final patch. In a perfect world each repositrory commit shouyld provide exactly one functionality - not more and not less. So your final patch should not reflect intermediate steps you took to implement some functionality because they are not really relevant. For the purpose of development and review it is fine to have a branch with lots of small commits but before merging you should just squash them into one.
I don't entirely agree. I personally find it very hard to review large patches as the amount of mental context generally seems to grow super-linearly in the amount of code touched. Moreover, I think it's important to remember that the need to read patches does not vanish the moment the patch is committed. To the contrary, review is merely the first of many instances in which a patch will be read. Other instances include, * when the patch is backported * when someone is trying to rebase one of their own changes on top of the patch * when another contributor is trying to follow the evolution of a piece of the compiler * when someone is later trying to understand a bug in the patch Consequently, I think it is fairly important not to throw out the structure that multiple, sensibly sized commits provides. Of course there is a compromise to be struck here: we don't want dozens of five-line patches; however I think that one mega-patch swings too far to the other extreme in the case of most features. Cheers, - Ben