Dataflow analysis for Cmm

[Ben Gamari]

Jan Stolarek <jan.stolarek at p.lodz.pl> writes:

>> (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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 454 bytes
Desc: not available
URL: <http://mail.haskell.org/pipermail/ghc-devs/attachments/20161020/1ea6d23b/attachment.sig>