Hello all, Is it intentional we have switched to 'Merge' commits for pull requests? e.g. https://github.com/wildfly/wildfly/commit/97ce5300f4277be023a112531b00dfa... Jason it is going to look like you have authored everything and in the future using annotate or browsing the history it is going to be more complex to identify why previous lines of code were written ;-) Regards, Darran Lofthouse. _______________________________________________ wildfly-dev mailing list wildfly-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/wildfly-dev

On 30/09/13 12:07, Tomaž Cerar wrote: > One of possible ways to do it is also merge commits, aka github's "merge > button" I wonder how would that help, since IIUC multiple PRs are now tested in a single batch to make sure the PRs together do not cause a regression, whereas each one passes the testsuite on its own. Then just push the whole batch…

Otherwise, I find merge commits messy and rarely bringing any value.

Rado _______________________________________________ wildfly-dev mailing list wildfly-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/wildfly-dev

On 9/30/13 10:04 AM, Jason Greene wrote: > > On Sep 30, 2013, at 7:11 AM, Radoslav Husar <rhusar(a)redhat.com&gt; wrote: > >> On 30/09/13 12:07, Tomaž Cerar wrote: >>> One of possible ways to do it is also merge commits, aka github's "merge >>> button" >> >> I wonder how would that help, since IIUC multiple PRs are now tested in >> a single batch to make sure the PRs together do not cause a regression, >> whereas each one passes the testsuite on its own. Then just push the >> whole batch… > > We would have to change the process to submit pull requests into a staging branch, and that branch once certified (all tests pass on all combination) would be pushed to the main master branch. In the event that a run fails, we would backup / redo the staging branch, and let it push again. > Currently processing multiple pull requests in a batch is just a matter of a script doing rebases in a loop against a special branch that starts out at master HEAD, testing the final result of that and then merging the final special branch. Ideally the same process could be used, just with merge commits in the loop instead of rebases. Is that another way of saying what you described above? Or is there a problem with the final merge of the special branch?

It's pretty important that this be efficient. The large majority of PRs get merged in sizable batches, because the heavy costs are the test execution time and the context switch on the part of the merger.

-- Brian Stansberry Principal Software Engineer JBoss by Red Hat _______________________________________________ wildfly-dev mailing list wildfly-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/wildfly-dev

On 9/30/13 10:59 AM, Jason Greene wrote: > > On Sep 30, 2013, at 10:19 AM, Brian Stansberry <brian.stansberry(a)redhat.com&gt; wrote: > >> On 9/30/13 10:04 AM, Jason Greene wrote: >>> >>> On Sep 30, 2013, at 7:11 AM, Radoslav Husar <rhusar(a)redhat.com&gt; wrote: >>> >>>> On 30/09/13 12:07, Tomaž Cerar wrote: >>>>> One of possible ways to do it is also merge commits, aka github's "merge >>>>> button" >>>> >>>> I wonder how would that help, since IIUC multiple PRs are now tested in >>>> a single batch to make sure the PRs together do not cause a regression, >>>> whereas each one passes the testsuite on its own. Then just push the >>>> whole batch… >>> >>> We would have to change the process to submit pull requests into a staging branch, and that branch once certified (all tests pass on all combination) would be pushed to the main master branch. In the event that a run fails, we would backup / redo the staging branch, and let it push again. >>> >> >> Currently processing multiple pull requests in a batch is just a matter >> of a script doing rebases in a loop against a special branch that starts >> out at master HEAD, testing the final result of that and then merging >> the final special branch. >> >> Ideally the same process could be used, just with merge commits in the >> loop instead of rebases. Is that another way of saying what you >> described above? Or is there a problem with the final merge of the >> special branch? > > Yes we could write something to mirror the old way and use merge commits and PR linking and so on. My suspicion is that the staging process would be faster because you just click the green button on "clean" changes. The test process could kick off automatically and email later, or to prevent duplicate runs we could just have an extra launch step (click the run button on brontes once you are done merging). > I don't understand what the staging process is.

How does PR linking relate to this?

>> It's pretty important that this be efficient. The large majority of PRs >> get merged in sizable batches, because the heavy costs are the test >> execution time and the context switch on the part of the merger. > > I agree thats the goal. > >> >> -- >> Brian Stansberry >> Principal Software Engineer >> JBoss by Red Hat >> _______________________________________________ >> wildfly-dev mailing list >> wildfly-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/wildfly-dev > > -- > Jason T. Greene > WildFly Lead / JBoss EAP Platform Architect > JBoss, a division of Red Hat > -- Brian Stansberry Principal Software Engineer JBoss by Red Hat

On Mon, Sep 30, 2013 at 6:26 PM, Jason Greene <jason.greene(a)redhat.com&gt; wrote: Think of it as everyone submitting pull requests into master-ignore instead of master, we just simply rename master-ignore to master-staging since they no longer ignore it. They start their feature branch on master (always on master), but then they submit a PR into master-staging, and then we click the green button if its good. If there is a conflict we either send it back, submit a new pull on their behalf, or do the merge offline (that part needs sorting). That would be bad, mostly because master-staging would/could get force pushes and if people would rebase against it before sending PR it would cause great problems.

On 9/30/13 11:26 AM, Jason Greene wrote: > > On Sep 30, 2013, at 11:17 AM, Brian Stansberry <brian.stansberry(a)redhat.com&gt; wrote: > >> On 9/30/13 10:59 AM, Jason Greene wrote: >>> >>> On Sep 30, 2013, at 10:19 AM, Brian Stansberry <brian.stansberry(a)redhat.com&gt; wrote: >>> >>>> On 9/30/13 10:04 AM, Jason Greene wrote: >>>>> >>>>> On Sep 30, 2013, at 7:11 AM, Radoslav Husar <rhusar(a)redhat.com&gt; wrote: >>>>> >>>>>> On 30/09/13 12:07, Tomaž Cerar wrote: >>>>>>> One of possible ways to do it is also merge commits, aka github's "merge >>>>>>> button" >>>>>> >>>>>> I wonder how would that help, since IIUC multiple PRs are now tested in >>>>>> a single batch to make sure the PRs together do not cause a regression, >>>>>> whereas each one passes the testsuite on its own. Then just push the >>>>>> whole batch… >>>>> >>>>> We would have to change the process to submit pull requests into a staging branch, and that branch once certified (all tests pass on all combination) would be pushed to the main master branch. In the event that a run fails, we would backup / redo the staging branch, and let it push again. >>>>> >>>> >>>> Currently processing multiple pull requests in a batch is just a matter >>>> of a script doing rebases in a loop against a special branch that starts >>>> out at master HEAD, testing the final result of that and then merging >>>> the final special branch. >>>> >>>> Ideally the same process could be used, just with merge commits in the >>>> loop instead of rebases. Is that another way of saying what you >>>> described above? Or is there a problem with the final merge of the >>>> special branch? >>> >>> Yes we could write something to mirror the old way and use merge commits and PR linking and so on. My suspicion is that the staging process would be faster because you just click the green button on "clean" changes. The test process could kick off automatically and email later, or to prevent duplicate runs we could just have an extra launch step (click the run button on brontes once you are done merging). >>> >> >> I don't understand what the staging process is. > > Think of it as everyone submitting pull requests into master-ignore instead of master, we just simply rename master-ignore to master-staging since they no longer ignore it. They start their feature branch on master (always on master), but then they submit a PR into master-staging, and then we click the green button if its good. If there is a conflict we either send it back, submit a new pull on their behalf, or do the merge offline (that part needs sorting). I don't get how this provides any sort of batching of processing. The github UI tells you the patch can merge cleanly, which is nice. And if the pull player tested it against the same commit as the current master, you know it's good to merge. But if master has progressed since the PR was tested (which is always the case with PR n>1 in a batch) then you haven't tested the combo. The batch testing is also where we get testing on Windows.

>> >> How does PR linking relate to this? > > Github generates pull request merge commits which nicely link to the original pull request e.g.: > https://github.com/wildfly/wildfly/compare/97ce5300f4...b3b54ad > OK, I thought you were talking about the JIRA Pull Request workflow. > On the merge commit I can click right into the original PR > > >> >>>> It's pretty important that this be efficient. The large majority of PRs >>>> get merged in sizable batches, because the heavy costs are the test >>>> execution time and the context switch on the part of the merger. >>> >>> I agree thats the goal. >>> >>>> >>>> -- >>>> Brian Stansberry >>>> Principal Software Engineer >>>> JBoss by Red Hat >>>> _______________________________________________ >>>> wildfly-dev mailing list >>>> wildfly-dev(a)lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev >>> >>> -- >>> Jason T. Greene >>> WildFly Lead / JBoss EAP Platform Architect >>> JBoss, a division of Red Hat >>> >> >> >> -- >> Brian Stansberry >> Principal Software Engineer >> JBoss by Red Hat > > -- > Jason T. Greene > WildFly Lead / JBoss EAP Platform Architect > JBoss, a division of Red Hat > -- Brian Stansberry Principal Software Engineer JBoss by Red Hat

On Sep 30, 2013, at 5:03 AM, Darran Lofthouse <darran.lofthouse(a)jboss.com&gt; wrote: > Hello all, > > Is it intentional we have switched to 'Merge' commits for pull requests? I'm exploring them as a way to speed up our merge process and provide better auditing. No decision yet. > > e.g. > > https://github.com/wildfly/wildfly/commit/97ce5300f4277be023a112531b00dfa... > > Jason it is going to look like you have authored everything and in the > future using annotate or browsing the history it is going to be more > complex to identify why previous lines of code were written ;-) That's not true. A merge commit links to the original author history. Github just displays the delta to be friendly. Git annotate also will show the original author sha1 and not the merge sha1.

-- Jason T. Greene WildFly Lead / JBoss EAP Platform Architect JBoss, a division of Red Hat