On 02/28/2017 08:48 AM, Vladimir Panteleev wrote:
On Tuesday, 28 February 2017 at 13:10:17 UTC, Andrei Alexandrescu wrote:
This would be the overwhelmingly frequent case.
...
This indicates a problem with the PR more often than not.
It is unfortunate that these two seem to be true right now, so given
that, I'll agree with you. Currently a lot of contributed code seems to
be placed in unnecessarily large commits with minimalist commit
messages, which would rank lather low on the quality scale of large
established projects. Ideally changes should be split into as many
commits as is reasonable, which by itself brings a lot of benefits, and
described in detail (50-char summary, long description of the change,
and change rationale). For examples, see the commit message guidelines
of Linux, git, or really any project that's large enough to have commit
message guidelines.
Thanks. I'd replace "changes should be split into as many commits as is
reasonable" with "changes should be split into as many pull requests as
is reasonable", which is a natural consequence of "most pull requests
should consist of one commit upon merging". (Of course there may be
several commits during PR review.)
One vs. several commits per merged pull request is a matter in which
reasonable people may disagree, and we can't do both ways. The
Foundation fosters that github pull requests are squashed upon merging,
with exceptions that need to be justified.
BTW we should put this in our guidelines, we have
https://wiki.dlang.org/Contributing_to_Phobos but not an equivalent
covering all of dlang properties. Is there one?
Thanks,
Andrei
