<div dir="ltr"><div>+1 to the review experience in Github being far worse than Phabricator, with basically all my specific concerns already being covered in this thread. I just wanted to add that our downstream LLVM port is based in a local Github Enterprise instance, and I find it far harder to review and respond to reviews there, compared to Phabricator. I'm not just opposed to change because I fear something new - I have active day-to-day experience with the something new, based on several years of experience, and I don't like it! I do acknowledge however, that some things have improved (e.g. multi-line commenting is now a thing, when it didn't used to be), so it's not an "absolutely never" from me, if the issues can be solved.</div><div><br></div><div>James<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 1 Oct 2021 at 04:11, Mehdi AMINI via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, Sep 30, 2021 at 8:05 PM Hubert Tong<br>
<<a href="mailto:hubert.reinterpretcast@gmail.com" target="_blank">hubert.reinterpretcast@gmail.com</a>> wrote:<br>
><br>
> On Thu, Sep 30, 2021 at 6:56 PM Mehdi AMINI via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> We talked about this with the IWG (Infrastructure Working Group) just<br>
>> last week coincidentally.<br>
>> Two major blocking tracks that were identified at the roundtable<br>
>> during the LLVM Dev Meeting exactly 2 years ago are still an issue<br>
>> today:<br>
>><br>
>> 1) Replacement for Herald rules. This is what allows us to subscribe<br>
>> and track new revisions or commits based on paths in the repo or other<br>
>> criteria. We could build a replacement based on GitHub action or any<br>
>> other kind of service, but this is a bit tricky (how do you store<br>
>> emails privately? etc.). I have looked around online but I didn't find<br>
>> another OSS project (or external company) providing a similar service<br>
>> for GitHub unfortunately, does anyone know of any?<br>
>><br>
>> 2) Support for stacked commits. I can see how to structure this<br>
>> somehow assuming we would push pull-request branches in the main repo<br>
>> (with one new commit per branch and cascading the pull-requests from<br>
>> one branch to the other), otherwise this will be a major regression<br>
>> compared to the current workflow.<br>
>><br>
>> What remains unknown to me is the current state of GitHub management<br>
>> of comments across `git commit --amend` and force push to update a<br>
>> branch.<br>
><br>
><br>
> Force pushing to a PR branch does make it harder for reviewers to see how review comments were addressed or what was done since they last looked at the PR. Are your use cases addressed if the workflow consists of pushing additional commits to address comments or pushing a merge commit to refresh the PR branch? When the PR is approved, the "squash and merge" option can be used to commit the patch as a single commit.<br>
<br>
This isn't compatible with stacked commits / stacked PR unfortunately.<br>
Also while merging main back into a branch of commits is "OK",<br>
rebasing multiple commits is much less friendly (the same conflict may<br>
have to be fixed over and over in each commit).<br>
<br>
> I find the code review experience in GitHub to be a productivity drain compared to Phabricator.<br>
><br>
> Older inline comments are much harder to find in GitHub.<br>
> Much more clicking needed in GitHub to actually load everything (blocks of comments folded away, comments collapsed not because you want them collapsed but because someone else or maybe just GitHub thought it should be collapsed, source files not loaded).<br>
> GitHub does not allow inline comments further away than a few lines from a change.<br>
<br>
Thanks! I have the same feeling, but I didn't have anything specific<br>
to point to and figured that this is in the scope of "I'll get used to<br>
it", but you mention some good points here.<br>
<br>
Best,<br>
<br>
-- <br>
Mehdi<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>