[jbosstools-dev] Block PRs for merge untill the Jenkins build is successful

Thu May 11 13:14:21 EDT 2017

Simple is beautiful. I fully agree

+1

On Thu, May 11, 2017 at 7:10 PM, Nick Boldt <nboldt at redhat.com> wrote:

> Better suggestion then:
>
> * Make sure if your PR requires a version bump that the PR includes TWO
> commits. One for the change, and one for the version bump. That way the
> pair of commits can be built in the PR build and verify it works, AND when
> cherry picking the commit across branches, you can pick only the change and
> not the version-bump commit too.
>
> So say we all?
>
> Nick
>
>
> On Wed, May 10, 2017 at 1:56 PM, Jean-Francois Maury <jmaury at redhat.com>
> wrote:
>
>> I would like to keep the version bump out of the pr because it will allow
>> us to cherry pick the pr in the maintenance branch
>>
>>
>>
>> Le 10 mai 2017 16:31, "Nick Boldt" <nboldt at redhat.com> a écrit :
>>
>>> I would rather see these PRs fail specifically because the versions were
>>> not correctly bumped.
>>> Then the PR could be fixed to include the correct bump and resubmitted /
>>> rebuilt.
>>> Or if in the intervening time another commit happened which fixed the
>>> versions, then the PR could be rebased and rebuilt.
>>>
>>> But having them fail is* A GOOD THING* as it reminds people to build
>>> locally before pushing the PR up to github. Doing so they'd see the
>>> baseline check fail locally and could therefore apply the version bump
>>> locally too.
>>>
>>> For example (and not to pick on anyone - this is just the most recent
>>> example I've seen) here's a commit that caused the build to break in both
>>> 4.4.x and master [1], which required 4 additional files [2] to be updated:
>>>
>>> [1] https://github.com/jbosstools/jbosstools-base/commit/3dafb28
>>> 7134414720de46da20c7e2f210cd73aaf
>>> [2] https://github.com/jbosstools/jbosstools-base/commit/dbfad28
>>> ff809f99e8c3bf3d80b26cd06806d3b2f
>>>
>>> You'll notice the PR [3] failed to build [4], too:
>>>
>>> [3] https://github.com/jbosstools/jbosstools-base/pull/579
>>> [4] https://dev-platform-jenkins.rhev-ci-vms.eng.rdu2.redhat.com
>>> /job/jbosstools-base-Pull-Request/121/console
>>>
>>> "[ERROR] Failed to execute goal org.eclipse.tycho.extras:tycho
>>> -p2-extras-plugin:1.0.0:compare-version-with-baselines
>>> (default) on project org.jboss.tools.usage:
>>> Only qualifier changed for (org.jboss.tools.usage/2.2.3.v20170505-1402).
>>>
>>> Expected to have bigger x.y.z than what is available in baseline
>>> (2.2.3.v20161213-1258)"
>>>
>>> So these failures SHOULD be respected and SHOULD block pushing the PR
>>> into the branch.
>>>
>>> *+100 for Dmitri's suggestion of implementing Protected Branches and
>>> Required Status Checks. *
>>>
>>> We can furthermore do what Fuse Tools does and require that PRs be
>>> reviewed before they can be merged, if we're ready to have that additional
>>> overhead on every PR.
>>>
>>>
>>>
>>> On Wed, May 10, 2017 at 3:17 AM, Dmitrii Bocharov <dbocharo at redhat.com>
>>> wrote:
>>>
>>>> Jeff, for this purpose we can think of some special comment for such
>>>> PRs, that would allow to merge them (like *testPR* for a new build).
>>>> As far as i know it's possible.
>>>>
>>>> On Wed, May 10, 2017 at 9:01 AM, Jean-Francois Maury <jmaury at redhat.com
>>>> > wrote:
>>>>
>>>>> I'm ok with that rule except for one case when the pr is done before
>>>>> the version bump has been merged then the Jenkins build will fail because
>>>>> of the baseline check so maybe we need to update the pr Jenkins build
>>>>>
>>>>> Jeff
>>>>>
>>>>> Le 9 mai 2017 23:10, "Mickael Istria" <mistria at redhat.com> a écrit :
>>>>>
>>>>>> FYI, not merging the broken patches is the policy followed by most
>>>>>> Eclipse.org projects and overall, none of this project has complained from
>>>>>> a reduced productivity; on the contrary, catching and fixing issues
>>>>>> immediately on the right context has improved quality and reduced the
>>>>>> necessary amount of quick fix patches (which are actually quite time
>>>>>> consuming and stressful for their low added-value).
>>>>>> So I think if it works for Eclipse.org projects, it can work for
>>>>>> JBoss Tools.
>>>>>>
>>>>>> Cheers,
>>>>>> Mickael
>>>>>>
>>>>>> _______________________________________________
>>>>>> jbosstools-dev mailing list
>>>>>> jbosstools-dev at lists.jboss.org
>>>>>> https://lists.jboss.org/mailman/listinfo/jbosstools-dev
>>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> jbosstools-dev mailing list
>>>> jbosstools-dev at lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/jbosstools-dev
>>>>
>>>
>>>
>>>
>>> --
>>>
>>> Nick Boldt
>>>
>>> Senior Software Engineer, RHCSA
>>>
>>> Productization Lead :: JBoss Tools & Dev Studio
>>>
>>> IM: @nickboldt / @nboldt / http://nick.divbyzero.com
>>> <https://red.ht/sig>
>>> TRIED. TESTED. TRUSTED. <https://redhat.com/trusted>
>>> @ @redhatnews <https://twitter.com/redhatnews>      Red Hat
>>> <https://www.facebook.com/RedHatInc>
>>>
>>
>
>
> --
>
> Nick Boldt
>
> Senior Software Engineer, RHCSA
>
> Productization Lead :: JBoss Tools & Dev Studio
>
> IM: @nickboldt / @nboldt / http://nick.divbyzero.com
> <https://red.ht/sig>
> TRIED. TESTED. TRUSTED. <https://redhat.com/trusted>
> @ @redhatnews <https://twitter.com/redhatnews>      Red Hat
> <https://www.facebook.com/RedHatInc>
>

-- 

JEFF MAURY

Red Hat

<https://www.redhat.com/>

jmaury at redhat.com
<https://red.ht/sig>
<https://redhat.com/summit>
@redhatjobs <https://twitter.com/redhatjobs>   redhatjobs
<https://www.facebook.com/redhatjobs/>   @redhatjobs
<https://instagram.com/redhatjobs>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/jbosstools-dev/attachments/20170511/9377a558/attachment.html 