[cfe-dev] [llvm-dev] Phabricator -> GitHub PRs?

[Hubert Tong via cfe-dev]

On Fri, Sep 11, 2020 at 6:53 PM David Blaikie <dblaikie at gmail.com> wrote:

> Is there any observable difference between "Squash and Merge" or "Rebase
> and Merge" when "enforce linear history" is enabled, then?
>
"Squash and Merge" will only generate one commit.


>
> On Fri, Sep 11, 2020 at 3:45 PM Stephen Neuendorffer via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Just to clarify: All the LLVM incubator repositories have "enforce linear
>> history" enabled.  Neither "Squash and Merge" or "Rebase and Merge" results
>> in a Merge commit in the git history.
>>
> I am not aware of/still have not experienced a case where "Rebase and
Merge" retains merge commits.


>
>> Steve
>>
>> On Fri, Sep 11, 2020 at 3:32 PM Hubert Tong <
>> hubert.reinterpretcast at gmail.com> wrote:
>>
>>> On Fri, Sep 11, 2020 at 6:12 PM Renato Golin <rengolin at gmail.com> wrote:
>>>
>>>> On Fri, 11 Sep 2020 at 19:32, Hubert Tong via llvm-dev <
>>>> llvm-dev at lists.llvm.org> wrote:
>>>>
>>>>> There seems to be a split between those who prefer to curate commits
>>>>>> locally and present them in the PR (i.e. Method 3) as they are to be
>>>>>> committed (i.e. squash/amend/etc from my workstation and push the result),
>>>>>> and those who seem to feel that it is better to avoid "git rebase" and "git
>>>>>> commit --amend" and let github handle rewriting the history with the commit.
>>>>>>
>>>>> Are these primarily developers or reviewers with these preferences?
>>>>> Are the people using Method 3 handling inline comments diligently? Is
>>>>> GitHub still "great" if Method 1 is the model with the least surprises?
>>>>>
>>>>
>>>> I don't think there's a clear difference between developers and
>>>> reviewers, we're all usually both at some point. I also tend to fluctuate
>>>> between manually curating commits to not caring much at all. I think if we
>>>> try to codify this with precision, no one will be satisfied. Worst still, a
>>>> lot of people will end up doing "the wrong thing" by accident and we'll all
>>>> get upset.
>>>>
>>>> On my current project, we all have very different views and styles, but
>>>> we reached a common consensus:
>>>> 1. No master rewrite, just like LLVM. This is madness, period.
>>>> 2. No branch merge commit into master. Just like LLVM. We
>>>> only thoroughly test the master branch.
>>>> 3. The original author decides if they want to rebase or squash at the
>>>> end.
>>>>
>>>> We're already lenient with commits and reverts, there's no point in
>>>> being tough just because we have a tool that allows us to do so.
>>>>
>>>> We already require people to write decent commit messages. This would
>>>> cover the review comments nicely: all comment reviews should be bundled in
>>>> one last commit, with a tag like "address review comments" or whatever and
>>>> a list of changes.
>>>>
>>>> Of course, if some review changes the series consistently, it could
>>>> give rise to a few more meaningful commits, with decent messages on their
>>>> own. No rules broken there.
>>>>
>>>> So, in a nutshell, review/approval like current phab,
>>>>
>>> Current phab has better diff-to-diff comparison than anything I've seen
>>> on GitHub. It even deals with merge-from-master/rebase-on-master fairly
>>> well. "Reviewing like current phab" matches "Method 1" more.
>>>
>>>
>>>> restrict to rebase/squash
>>>>
>>> As in no final "merge". I'm fine with that.
>>> I'll note that "rebase" is a departure from current phab in that a
>>> single review creates multiple commits. Not that I am opposed to such
>>> commits, but I am concerned with continual force pushes to PR branches. A
>>> "final history clean-up" after reviews are done is okay in my book.
>>>
>>>
>>>> , let people decide, continue encouraging decent commit messages and
>>>> squashing final review changes.
>>>>
>>> From the above, I take it to mean that this "squashing" you mean last
>>> here is about rewriting the history (if not using GitHub's "squash and
>>> merge") before using GitHub's "rebase and merge"?
>>>
>>>
>>>> cheers,
>>>> --renato
>>>>
>>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200911/a705ceda/attachment-0001.html>