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

[Nick Boldt]

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/3dafb287134414720de46da20c7e2f210cd73aaf
[2]
https://github.com/jbosstools/jbosstools-base/commit/dbfad28ff809f99e8c3bf3d80b26cd06806d3b2f

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>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/jbosstools-dev/attachments/20170510/a733fe53/attachment.html