[systemd-devel] [ANNOUNCE] Git development moved to github

[Filipe Brandenburger]

On Wed, Jun 10, 2015 at 9:52 AM, Lennart Poettering
<lennart at poettering.net> wrote:
> On Wed, 10.06.15 08:25, Filipe Brandenburger (filbranden at google.com) wrote:
>> On Wed, Jun 10, 2015 at 6:31 AM, Alban Crequy <alban at endocode.com> wrote:
>> > FWIW it only loses the comments if people comment on individual
>> > commits instead of commenting on the "Files changed" tab of a PR. I
>> > usually comment in this way on purpose instead of commenting on
>> > commits, so that the history of comments are kept in the PR, even
>> > after rebase (it might be folded if the chunk of the patch is not
>> > there anymore, but the comment is still in the PR). If you really want
>> > to comment on an individual commit (but I don't recommend it), you can
>> > include the reference of the PR in your comment (#42), then github
>> > will keep your comment attached to the PR.
>>
>> Ah that makes sense!
>>
>> Indeed as I explained I like to look at the individual commits, so
>> that would explain why my comments would get lost as a new version is
>> pushed...
>>
>> > I think it is fine as it is as long as people comment in the "Files
>> > changed" tab.
>>
>> Lennart, do you think setting that rule is better than the "one PR per
>> version of patchset"?
>
> No. We should review commits, not diffs. We also should review commit
> msgs. (see other mail)

Another downside of adding comments to the commits is that e-mail
notifications are not sent for them (I just noticed that while lurking
on #164, I got e-mails for the main thread but not for Lennart's
comments on commit 5f33680.)

I think adding comments on the "Files changed" would work on cases such as:

1) The PR contains only a single commit, in which case the diff in
"Files changed" will match the commit itself. (You still need to look
at the commit description, but even if you do it from the "Commits"
tab you can't really add any line comments directly to it anyways.)

2) If the commits change disjoint sets of files (you could check that
first, and then review the code in the "Files changed" tab.)

I think the exception is when a PR is both introducing new code and
later changing it in a follow up commit but I guess that's not really
too frequent (though I'm clearly guilty of it on #44.)

Can we try to add comments to "Files changed"? Not asking not to look
at the commits, yes looking at the commits is important! It's just
that I think if we could have the e-mail notifications for the line
comments, make sure they are kept in the same thread and be able to
keep multiple versions of a patchset around in the same PR (instead of
the wonky PR linking) I think that would be a huge win... We can
always fall back to opening a new PR and closing the old one, but I'd
prefer if that was the exception and not the rule...

It really sounds like what you really want is Gerrit... I think
gerrithub.io (which I haven't tried personally) might be what bridges
these two worlds... Makes it easy for the submitters to send you
commits, makes it easy for the reviewers to adopt the new code, tracks
pending requests.

Cheers!
Filipe