llvm dev - Jun 2016 - [cfe-dev] [lldb-dev] GitHub anyone?

I'd suggest a workflow like the following:

- developer commits locally to a feature/bug dev branch. You can commit
work in progress, experiments, have bad commit messages etc

- developer commits locally to a feature/bug release branch. Tidy up into a
small number of logical commits, nice messages etc. I'd suggest it's
good
to have at least two commits: 1) a commit adding a test that fails, and is
marked as expected to fail, demonstrating the bug or lack of feature. 2) a
commit that fixes the bug or adds the feature, and marks the test as
expected to pass.

- developer rebases to master and tests

- developer pushes their feature/bug release branch to their github fork of
llvm, issues a pull request

- the appropriate maintainer (or or automatic system) causes build and
tests to be run on the proposed bug fix.

- if the tests work, then do a "git merge --no-ff" to master

There's room to discuss the last part. That gives a master history with
exactly one commit per feature or bug fix. The details of how it was done
are on a different branch that merges back.

You might prefer merge --ff-only. This means that the merge can only happen
if the new feature has been rebased to the head of master. The commits in
the new feature each become a commit in master. In this case you should
make very sure that every commit works -- which is defined as no crashes;
tests expected to work, work; tests expected to fail, fail.


On Mon, Jun 6, 2016 at 8:07 PM, James Y Knight via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> +1 to that. I would strongly suggest that we continue to commit to master
> first, like we've always done in llvm.
>
> On Jun 6, 2016 11:44 AM, "Joerg Sonnenberger via cfe-dev" <
> cfe-dev at lists.llvm.org> wrote:
>
>> On Mon, Jun 06, 2016 at 10:32:45AM -0500, via llvm-dev wrote:
>> > My only hesitation with this is that this requires use of
cherry-pick,
>> > which is not idea.  The way most git repositories work is to put
>> > everything that should go into a release branch in the release
branch
>> > *first* and then merge the release branch to master, ensuring that
>> > everything going out in a release will make it into the next
release.
>> > This is how the gitflow workflow works, for example.
>>
>> The model of "commit to oldest first" is IMO one of the most
stupid
>> concepts I have ever seen in git "workflows". It is contrary
to the way
>> software development works and essentially just a bad workaround for
>> broken cherry picks. I've seen more than one project starting to
use
>> this model due to advocacy after deciding to use git, stumble around
>> with it for a release or two and then going back to a normal release
>> management approach. Even the argument you have presented here does not
>> make sense to me. Just because a change has been committed to a release
>> branch, doesn't mean it won't get replaced later.
>>
>> Joerg
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160606/62599d58/attachment.html>

On 6 Jun 2016 12:52 p.m., "Bruce Hoult via cfe-dev" <cfe-dev at
lists.llvm.org>
wrote:
>
> I'd suggest a workflow like the following:
>
> - developer commits locally to a feature/bug dev branch. You can commit
work in progress, experiments, have bad commit messages
etc
>
> - developer commits locally to a feature/bug release branch. Tidy up into
a small number of logical commits, nice messages etc. I'd suggest it's
good
to have at least two commits: 1) a commit adding a test that fails, and is
marked as expected to fail, demonstrating the bug or lack of feature. 2) a
commit that fixes the bug or adds the feature, and marks the test as
expected to pass.

Our policy and workflow is to upstream small, incremental changes wherever
possible (rather than presenting reviewers with a complete, monolithic
change to review), and that seems to conflict somewhat with your suggestion
of working on feature branches. But so long as the changes we receive for
review are sufficiently small and authors are happy to receive feedback
that the design of the first patch in a series of N is problematic (so the
whole series needs rework), I don't think we need to care how they produced
the patch.
> - developer rebases to master and tests
>
> - developer pushes their feature/bug release branch to their github fork
of llvm, issues a pull request
>
> - the appropriate maintainer (or or automatic system) causes build and
tests to be run on the proposed bug fix.
>
> - if the tests work, then do a "git merge --no-ff" to master
>
> There's room to discuss the last part. That gives a master history with
exactly one commit per feature or bug fix. The details of how it was done
are on a different branch that merges back.

I assume you're simplifying for exposition; that's not really how git
works
(commits aren't associated with any particular branch except perhaps by a
convention of annotations in the commit message; a branch is just a label
for its current head commit). More precisely, what this would give us is a
linear one-commit-per-merge history *if we follow the first parent of each
commit from master*. I think that should be fine in practice, but it might
be better to instead only allow a commit to be pushed to master on github
if its first parent is the current master; that would allow us to avoid
merge commits in most cases.
> You might prefer merge --ff-only. This means that the merge can only
happen if the new feature has been rebased to the head of master. The
commits in the new feature each become a commit in master. In this case you
should make very sure that every commit works -- which is defined as no
crashes; tests expected to work, work; tests expected to fail,
fail.
>
>
> On Mon, Jun 6, 2016 at 8:07 PM, James Y Knight via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
>>
>> +1 to that. I would strongly suggest that we continue to commit to
master first, like we've always done in llvm.
>>
>>
>> On Jun 6, 2016 11:44 AM, "Joerg Sonnenberger via cfe-dev"
<
cfe-dev at lists.llvm.org> wrote:
>>>
>>> On Mon, Jun 06, 2016 at 10:32:45AM -0500, via llvm-dev wrote:
>>> > My only hesitation with this is that this requires use of
cherry-pick,
>>> > which is not idea.  The way most git repositories work is to
put
>>> > everything that should go into a release branch in the release
branch
>>> > *first* and then merge the release branch to master, ensuring
that
>>> > everything going out in a release will make it into the next
release.
>>> > This is how the gitflow workflow works, for example.
>>>
>>> The model of "commit to oldest first" is IMO one of the
most stupid
>>> concepts I have ever seen in git "workflows". It is
contrary to the way
>>> software development works and essentially just a bad workaround
for
>>> broken cherry picks. I've seen more than one project starting
to use
>>> this model due to advocacy after deciding to use git, stumble
around
>>> with it for a release or two and then going back to a normal
release
>>> management approach. Even the argument you have presented here does
not
>>> make sense to me. Just because a change has been committed to a
release
>>> branch, doesn't mean it won't get replaced later.
>>>
>>> Joerg
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160606/b632d82e/attachment.html>

> On Jun 6, 2016, at 1:29 PM, Richard Smith via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> On 6 Jun 2016 12:52 p.m., "Bruce Hoult via cfe-dev" <cfe-dev
at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
> >
> > I'd suggest a workflow like the following:
> >
> > - developer commits locally to a feature/bug dev branch. You can
commit work in progress, experiments, have bad commit messages etc
> >
> > - developer commits locally to a feature/bug release branch. Tidy up
into a small number of logical commits, nice messages etc. I'd suggest
it's good to have at least two commits: 1) a commit adding a test that
fails, and is marked as expected to fail, demonstrating the bug or lack of
feature. 2) a commit that fixes the bug or adds the feature, and marks the test
as expected to pass
> 
While I applaud small patches, this is going too far in my opinion. In my
opinion the system should be designed to ease reading and understanding the
changes (since a change is usually read by more people than it is written...).
In this specific instance I really like the fact to have the testcase together
with the commit that fixes it because it often present a short understandable
and concrete example of the issue getting fixed [1]

- Matthias

[1] I should mention here that we have a number of testcases in the repository
that fail this crtiterium! Cases where the test is just extracted from a real
world input but still a lot of instructions and information irrelevant to the
problem. Just because a testcase is bugpoint-reduced does not mean it is
minimal!
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160606/bd17cc89/attachment.html>