Thoughts on the Contributing page

[Tobias Dammers]

On Sat, Jan 19, 2019 at 03:55:06PM -0500, Ben Gamari wrote:
> 
> 
> For those following along at home:
> 
>    David has been looking at revising our contributor documentation.
>    He has started consolidating a variety of relevant content on the
>    Contributing page of the Wiki [1]. Below are my thoughts; feel free
>    to chime in with your own.
> 
> 
> David,
> 
> The contributing page is looking quite good. However, I do wonder
> whether we could reduce the link "fan-out" a bit more: it still rather
> feels like a collection of links with no clear "beginning".

> 
> The "Newcomers to GHC" section is a great start but I see two
> potential issues:
> 
>  * we don't clearly articulate the precise steps that a newcomer will
>    need to take
> 
>  * the first thing we mention are four links which, while useful,
>    constitute a significant volume of reading for a newcomer.
>    I fear we may lose people at this point.

Agree; fan-out superficially seems like a good idea (more information is
good, right?), but it just ends up overwhelming and confusing people
here. More information can be linked to later; the "Newcomers' Guide"
should have a clear narrative, with highly opinionated, well-tested
steps that will lead you from "I have nothing" to "Baby's First Merge
Request" and essentially Just Work.

> Regarding the second point, I think that WorkingConventions/FixingBugs is a
> good model in that it clearly lists a series of concrete steps that the
> contributor should take. Admittedly, I think some of the detail should
> be dropped or moved (e.g. the mention of setting git's user.name
> variable is likely unnecessary in 2019).
> 
> I think that something similar to this list should be the first thing
> one sees when they reach the contributing page. Ideally the "typical"
> case of a new contributor wanting to submit their first patch would be
> able to gather everything they need to get started in one or two
> screenfuls of text. After that prose can come additional links to
> more in-depth documentation.

Exactly.

> 
> Other various issues I noticed:
> 
>  * We link to wiki:WorkingConventions from a variety of places
>    (including wiki:Contributing) but it is now just a link to
>    wiki:Contributing.

>  * wiki:WorkingConventions/FixingBugs still has references to
>    Phabricator. In general we should start culling such references.
> 
>  * wiki:WorkingConventions/FixingBugs also has references to Trac
>    documentation. We should try to replace these with the relevant
>    GitLab documentation when we migrate.
> 
>  * wiki:WorkingConventions/FixingBugs should be updated to reflect
>    that GitLab CI is now the source of truth for validation.

A good first step, I think, would be to simply grep the entire wiki for
terms like "phab", "trac", etc., which is going to be significantly
easier after the migration due to gitlab's wiki being a git repo of
markdown files, rather than SQL-backed. If we're pressed for man-hours,
it might even be best to just delete outdated pages, and evolve new
material as the need arises.

> 
>  * I don't know exactly how this should look but I think we need to do a
>    better job of concisely stating what we expect of contributions. That
>    is:
> 
>      * commit messages should be readable and discuss what the patch
>        does. "Fixes #NNNN" is not an adequate commit message.
> 
>      * changes should be commented where necessary (my usual rule of
>        thumb is "write the comment you would have liked to read when you
>        started writing your patch"). We should mention the Note
>        convention.
> 
>      * commit messages and comments should refer to ticket numbers where
>        appropriate (e.g. using usual #NNNN syntax; people often get this
>        wrong).
> 
>      * commits should be either squashed or logically distinct,
>        individually buildable changes.
> 
>      * changes to most of `base` need to go through the CLC (this may be
>        optional as defining what set of `base` this applies to is a bit
>        tricky; in the interest of keeping things concise we may be
>        better off handling this personally on a case-by-case basis)
> 
>      * code changes should conform to GHC's (somewhat fluid, for better
>        or worse) code style
>        (https://ghc.haskell.org/trac/ghc/wiki/Commentary/CodingStyle)
> 
>      * thought should be given to testing
> 
>     Having a document which succinctly summarised these expectations as
>     we could easily refer to it during code review. Even better, we
>     could excerpt it as a checklist in our merge request description
>     template (I have put up an initial attempt at this as !149).

I believe this should be a separate, self-contained document; we should
not overload the New Contributors' Guide with this information, nor
should it be scattered across the rest of the documentation (though more
pages should exist to go into full detail on each of those points). The
New Contributors' Guide should, however, follow these practices and thus
document them by example. E.g., the tutorial should include a step where
you squash your commits and add suitable comments. Likewise, the NOTE:
convention can be introduced simply by walking through the steps as part
of the tutorial.

On a completely unrelated note, I was thinking that it may be feasible
and desirable to hack up a quick broken-links check for the wiki,
possibly even as part of the migration script (which has to extract wiki
links anyway as it goes, and also visits every wiki page at some point,
so maintaining a list of links and then crossing off the ones that exist
shouldn't be a huge effort). Armed with such a list, we could then
semi-automatically deal with broken links and redirects as part of a
cleanup sweep. I'll have to give redirects a bit more thought in this
context though.