Re: [lldb-dev] [llvm-dev] [cfe-dev] [Github] RFC: linear history vs merge commits

[Jameson Nash via lldb-dev]

> The way it decides that there are "outdated comments" and that those
comments should be hard to does not win it any points in my book (Hubert
Tong)

IIUC, Github has recently changed this, and is now more like Phabricator.
(in that it now seems to add a mark to say the diff is likely outdated, but
has a separate "Resolve Conversation" button). That said, this might bring
up a different incentive for staying with Phabricator, which is that Github
can and does make minor changes to their PR workflow over time, while LLVM
would retain more control over their Phabricator instance.


> Full disclosure: "Revert" commits also wreak havoc on "git bisect" and
"git blame"

As an outsider, I'm not sure I fully understand the comments about `git
bisect` (and blame) not being able to deal with merge and revert commits.
Having worked mostly on the JuliaLang project (which does not discourage
merge commits), we've not typically had much difficulty with either. The
bisect tool is intelligent about being able to isolate which branch of the
graph introduced (or fixed) the behavior of interest, and can isolate
within that branch which commit first show the change. The presence of a
revert commit can be a minor speed-bump (especially if it means the build
previously failed), but "wreak havoc" seems like a bit of hyperbole for
needing to call git blame again—and doesn't seem much different from how
svn would necessarily handle the same situation?

One point that may be worth mentioning though is that merge commits are
also themselves unrestricted changes(*), which can be an interesting gotcha
(it's hard to represent this diff). So if merges are allowed in the
timeline, LLVM may additionally decide whether to permit human-assisted
merges, or to ask that developers rebase the branch first in that case.
(*In git, each commit is a record of the state of all contents of the repo,
plus one or more parent links to the previous state. A merge commit is
distinct only in that it has multiple parents that are indicated as
contributing in some way to the final state, but they aren't otherwise
restricted in what gets changed.)

There is however a philosophic difference of various git coding styles that
LLVM can chose. One school of thought suggests that you should never rebase
code, and instead frequently merge branches in all directions. This gives
the advantage that a `git bisect` attempt will be considering the code as
originally written, and so the result of `git bisect` or `blame` may be
more likely to tell whether a later discovered issue (or other archeology
reason) arose from an issue with the original commit, or arose as a result
of a conflict with a simultaneous change.

A counter school of thought instead argues for preserving a simpler (or in
the limit, a linear) ordering of all commits. I won't say as much about
this, since I think the benefits of this are already obvious to a community
that already requires it.

I also won't say that one school is better or worse than the other (I
happen to use a hybrid in most of my projects). There are also other
approaches to commit management I haven't mentioned, but I wanted to at
least share some thoughts, in the hopes that someone might find the
information useful.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] [llvm-dev] [cfe-dev] [Github] RFC: linear history vs merge commits

[Tom Stellard via lldb-dev]

On 01/31/2019 09:51 AM, Roman Lebedev via llvm-dev wrote:
> On Thu, Jan 31, 2019 at 8:29 PM David Greene via cfe-dev
>  wrote:
>>
>> Mehdi AMINI  writes:
>>
>>> What is the practical plan to enforce the lack of merges? When we
>>> looked into this GitHub would not support this unless also forcing
>>> every change to go through a pull request (i.e. no pre-receive hooks
>>> on direct push to master were possible). Did this change? Are we
>>> hoping to get support from GitHub on this?
>>>
>>> We may write this rule in the developer guide, but I fear it'll be
>>> hard to enforce in practice.
>>
>> Again, changes aren't going through git yet, right?  Not until SVN is
>> decommissioned late this year (or early next).  SVN requires a strict
>> linear history so it handles the enforcement for now.
> 
>> My personal opinion is that when SVN is decomissioned we should use pull
>> requests, simply because that's what's familiar to the very large
>> development community outside LLVM.  It will lower the bar to entry for
>> new contributors.  This has all sorts of implications we need to discuss
>> of course, but we have some time to do that.
> My personal opinion is an opposite of that one.
> 
> *Does* LLVM want to switch from phabricator to github pr's?
> I personally don't recall previous discussions.
> Personally, i hope not, i hope phabricator should/will stay.
> 

This hasn't been decided yet, but this is a whole other discussion
that deserves it own thread (not saying we need to decide this now though).

-Tom

> Low bar for entry is good, but not at the cost of raising the bar
> for the existing development community.
> In particular, i'm saying that github's ui/workflow/feature set
> is inferior to that one of phabricator.
> 
> Is the low entry bar the only benefit?
> While it is good, it should not be the only factor.
> The contributors will already need to know how to build LLVM,
> write tests, make meaningful changes to the code.
> Additionally having to know how to work with phabricator
> isn't that much to ask for, considering the other prerequisites..
> 
>> If we don't use PRs, then we're pretty much on the honor system to
>> disallow merges AFAIK.
>>
>>  -David
> Roman.
> 
>> ___
>> cfe-dev mailing list
>> cfe-...@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> ___
> LLVM Developers mailing list
> llvm-...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> 

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] [llvm-dev] [cfe-dev] [Github] RFC: linear history vs merge commits

[Tom Stellard via lldb-dev]

On 01/30/2019 10:53 PM, Mehdi AMINI via llvm-dev wrote:
> 
> 
> On Wed, Jan 30, 2019 at 1:19 PM Eric Christopher via cfe-dev 
> mailto:cfe-...@lists.llvm.org>> wrote:
> 
> On Wed, Jan 30, 2019 at 12:42 PM David Greene via cfe-dev
> mailto:cfe-...@lists.llvm.org>> wrote:
> >
> > Bruce Hoult via llvm-dev  > writes:
> >
> > > How about:
> > >
> > > Require a rebase, followed by git merge --no-ff
> > >
> > > This creates a linear history, but with extra null merge commits
> > > delimiting each related series of patches.
> > >
> > > I believe it is compatible with bisect.
> > >
> > > https://linuxhint.com/git_merge_noff_option/
> >
> > We've done both and I personally prefer the strict linear history by a
> > lot.  It's just much easier to understand a linear history.
> >
> 
> Agreed. Let's go with option #1.
> 
> 
> What is the practical plan to enforce the lack of merges? When we looked into 
> this GitHub would not support this unless also forcing every change to go 
> through a pull request (i.e. no pre-receive hooks on direct push to master 
> were possible). Did this change? Are we hoping to get support from GitHub on 
> this?
> 

No enforcement plan at this point, but I was planning to contact github about 
this to
see what options we had.  Last time you looked into it, did you talk to anyone 
at github
support?

This is also why I think it's important to decide early so we have time to look 
at
what our options are to enforce these policies. If pull requests are our only
option for enforcement, then I think it would be good to know that before we
have a large debate about Phabricator vs Pull Requests.

-Tom


> We may write this rule in the developer guide, but I fear it'll be hard to 
> enforce in practice.
>
 -- 
> Mehdi
> 
> 
> 
> ___
> LLVM Developers mailing list
> llvm-...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> 

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] [llvm-dev] [cfe-dev] [Github] RFC: linear history vs merge commits

[Jeremy Lakeman via lldb-dev]

https://help.github.com/articles/checking-out-pull-requests-locally/

Script a bot to scrape the patches and post to phabricator?

On Fri, 1 Feb 2019 at 04:22, Roman Lebedev via llvm-dev <
llvm-...@lists.llvm.org> wrote:

> On Thu, Jan 31, 2019 at 8:29 PM David Greene via cfe-dev
>  wrote:
> >
> > Mehdi AMINI  writes:
> >
> > > What is the practical plan to enforce the lack of merges? When we
> > > looked into this GitHub would not support this unless also forcing
> > > every change to go through a pull request (i.e. no pre-receive hooks
> > > on direct push to master were possible). Did this change? Are we
> > > hoping to get support from GitHub on this?
> > >
> > > We may write this rule in the developer guide, but I fear it'll be
> > > hard to enforce in practice.
> >
> > Again, changes aren't going through git yet, right?  Not until SVN is
> > decommissioned late this year (or early next).  SVN requires a strict
> > linear history so it handles the enforcement for now.
>
> > My personal opinion is that when SVN is decomissioned we should use pull
> > requests, simply because that's what's familiar to the very large
> > development community outside LLVM.  It will lower the bar to entry for
> > new contributors.  This has all sorts of implications we need to discuss
> > of course, but we have some time to do that.
> My personal opinion is an opposite of that one.
>
> *Does* LLVM want to switch from phabricator to github pr's?
> I personally don't recall previous discussions.
> Personally, i hope not, i hope phabricator should/will stay.
>
> Low bar for entry is good, but not at the cost of raising the bar
> for the existing development community.
> In particular, i'm saying that github's ui/workflow/feature set
> is inferior to that one of phabricator.
>
> Is the low entry bar the only benefit?
> While it is good, it should not be the only factor.
> The contributors will already need to know how to build LLVM,
> write tests, make meaningful changes to the code.
> Additionally having to know how to work with phabricator
> isn't that much to ask for, considering the other prerequisites..
>
> > If we don't use PRs, then we're pretty much on the honor system to
> > disallow merges AFAIK.
> >
> >  -David
> Roman.
>
> > ___
> > cfe-dev mailing list
> > cfe-...@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> ___
> LLVM Developers mailing list
> llvm-...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev