[systemd-devel] [ANNOUNCE] Git development moved to github
Ronny Chevalier
chevalier.ronny at gmail.com
Thu Jun 11 12:31:01 PDT 2015
On Thu, Jun 11, 2015 at 6:31 PM, Filipe Brandenburger
<filbranden at google.com> wrote:
> 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.)
Yes you need to specify for each PR you are interested in that you
want to receive mail notifications for the PR... (I think it's the
subscribe button at the bottom)
>
> 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
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
More information about the systemd-devel
mailing list