New candidate JEP: 369: Migrate to GitHub

Florian Weimer fw at deneb.enyo.de
Thu Nov 14 19:56:52 UTC 2019


* Omair Majid:

> Hi,
>
> Florian Weimer <fw at deneb.enyo.de> writes:
>
>> What happens to previous commentary when the PR is updated based on
>> feedback?  That's one of the things that's quite confusing in Gerrit.
>>
>> If you make a change and the diff hunk to which previous comments have
>> been applied is gone, does that mean the comments' concerns have been
>> addressed?  It is impossible to know automatically.  Gerrit does not
>> automatically remove or close the comments, but hides them from the
>> default view (i.e., there is no visual cue that there are unresolved
>> comments on a previous version), which neatly reflects the inherent
>> ambiguity of the situation, but is also not very helpful.
>
> I can speak to this particular issue in github.
>
> When you comment on a line in the github web UI, and then rebase +
> force-push the commit so the line (or file!) isn't even there, github
> still keeps that comment and sticks an "outdated" label in that comment.
>
> Here is an example. I created a new file in a PR, left a comment to help
> reviewers, and then renamed the file. The comment still stays, but has
> an "Outdated" label until I (or someone else) hits the "Resolve
> conversation" button in the PR.
>
> https://github.com/dotnet/source-build/pull/1300#pullrequestreview-315785658
>
> If you try and follow the comment to the (now gone) change in code,
> though, github will say it can't find the change that the comment refers
> to. Hopefully there's enough context in next to the comment to figure
> things out.

So this then only appears in the linearized pull request history, but
not in the commit view.  So at least it's not completely gone, which
isn't too bad.  It seems on par with any email-based solution.

> Does that help address your concern?

But now I have another one: Your email client replaced
<fw at deneb.enyo.de> with <fweimer at redhat.com>, which I find … odd.


More information about the discuss mailing list