Request: Phab Differentials should include road maps
Hi devs, I have what I hope is a simple request: that patch submissions contain a "road map" describing the patch. I'll illustrate via example: I just took a quick look at D323, about updating the design of Uniques. Although this patch was fairly straightforward, I would have been helped by a comment somewhere saying "All the important changes are in Unique.lhs. The rest of the changes are simply propagating the new UniqueDomain type." Then, I would just look at the one file and skim the rest very briefly. The reason I'm requesting this comment from the patch author is that my assumption above -- that all the action is in Unique.lhs -- might be quite wrong. Maybe there's a really important (perhaps one-line) change elsewhere that deserves attention. Or, maybe there's a function/type in Unique.lhs that the patch author is very uncertain about and wants extra scrutiny. In any case, a few sentences at the top of the patch would help focus reviewers' time where the author thinks it is most needed. What do we think? Is this a behavior we wish to adopt? Thanks! Richard
Hi, Am Dienstag, den 14.10.2014, 09:33 -0400 schrieb Richard Eisenberg:
What do we think? Is this a behavior we wish to adopt?
sounds sensible. I only worry that if people put such comments in the DR description, they will end up in the commit pushed to the tree, if done using "arc land". Maybe the submitter should add such things as comments to the DR? Greetings, Joachim -- Joachim “nomeata” Breitner mail@joachim-breitner.de • http://www.joachim-breitner.de/ Jabber: nomeata@joachim-breitner.de • GPG-Key: 0xF0FBF51F Debian Developer: nomeata@debian.org
I frequently find myself asking for a different kind of road map: a wiki page saying - what is the problem we are trying to solve - what is the general approach for solving it - what is the specification for what a GHC user (or maybe a GHC API client, depending) would see? - what is a road map for how the implementation is structured. We often have these wiki pages but not always. Simply reviewing a big blob of source-code diffs and trying to reconstruct the above four points is not much fun! Moreover the act of writing them can be fantastically illuminating. The StaticPtr stuff is a case in point. Simon | -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of | Richard Eisenberg | Sent: 14 October 2014 14:34 | To: ghc-devs@haskell.org Devs | Subject: Request: Phab Differentials should include road maps | | Hi devs, | | I have what I hope is a simple request: that patch submissions contain | a "road map" describing the patch. I'll illustrate via example: I just | took a quick look at D323, about updating the design of Uniques. | Although this patch was fairly straightforward, I would have been | helped by a comment somewhere saying "All the important changes are in | Unique.lhs. The rest of the changes are simply propagating the new | UniqueDomain type." Then, I would just look at the one file and skim | the rest very briefly. The reason I'm requesting this comment from the | patch author is that my assumption above -- that all the action is in | Unique.lhs -- might be quite wrong. Maybe there's a really important | (perhaps one-line) change elsewhere that deserves attention. Or, maybe | there's a function/type in Unique.lhs that the patch author is very | uncertain about and wants extra scrutiny. In any case, a few sentences | at the top of the patch would help focus reviewers' time where the | author thinks it is most neede d. | | What do we think? Is this a behavior we wish to adopt? | | Thanks! | Richard | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs
Yes, I think what Richard wants is point #4 out of your list Simon:
how you might follow the patch implementing it. I think this is a good
idea, actually, and would help digest some patches more quickly.
Richard, I just had a talk with Phabricator upstream about this, and I
think this is certainly doable and can be added to our list of
extensions.
Here's how I would imagine what it would look like: Below the
'Summary' field, there is the 'Test Plan' field (as in D344). We can
add another field, 'Patch Roadmap', that appears the same way (i.e. a
bulk textedit form) and appears in the same area as well. How does
that sound?
On Tue, Oct 14, 2014 at 9:09 AM, Simon Peyton Jones
I frequently find myself asking for a different kind of road map: a wiki page saying - what is the problem we are trying to solve - what is the general approach for solving it - what is the specification for what a GHC user (or maybe a GHC API client, depending) would see? - what is a road map for how the implementation is structured.
We often have these wiki pages but not always. Simply reviewing a big blob of source-code diffs and trying to reconstruct the above four points is not much fun! Moreover the act of writing them can be fantastically illuminating. The StaticPtr stuff is a case in point.
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of | Richard Eisenberg | Sent: 14 October 2014 14:34 | To: ghc-devs@haskell.org Devs | Subject: Request: Phab Differentials should include road maps | | Hi devs, | | I have what I hope is a simple request: that patch submissions contain | a "road map" describing the patch. I'll illustrate via example: I just | took a quick look at D323, about updating the design of Uniques. | Although this patch was fairly straightforward, I would have been | helped by a comment somewhere saying "All the important changes are in | Unique.lhs. The rest of the changes are simply propagating the new | UniqueDomain type." Then, I would just look at the one file and skim | the rest very briefly. The reason I'm requesting this comment from the | patch author is that my assumption above -- that all the action is in | Unique.lhs -- might be quite wrong. Maybe there's a really important | (perhaps one-line) change elsewhere that deserves attention. Or, maybe | there's a function/type in Unique.lhs that the patch author is very | uncertain about and wants extra scrutiny. In any case, a few sentences | at the top of the patch would help focus reviewers' time where the | author thinks it is most neede d. | | What do we think? Is this a behavior we wish to adopt? | | Thanks! | Richard | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards, Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/
On Oct 17, 2014, at 12:32 PM, Austin Seipp
Here's how I would imagine what it would look like: Below the 'Summary' field, there is the 'Test Plan' field (as in D344). We can add another field, 'Patch Roadmap', that appears the same way (i.e. a bulk textedit form) and appears in the same area as well. How does that sound?
Sounds perfect to me. I was actually thinking of your proposed approach, but didn't want to push for it as it requires more work than social enforcement would. Thanks, Richard
participants (4)
-
Austin Seipp -
Joachim Breitner -
Richard Eisenberg -
Simon Peyton Jones