Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix JENKINS-51638 - allow lower case 'default' UserMergeStrategy #711

Merged

Conversation

MarkEWaite
Copy link
Contributor

@MarkEWaite MarkEWaite commented Apr 26, 2019

JENKINS-56138 - Handle lower case 'default' UserMergeStrategy value

Some users upgrading from prior git plugin versions to git plugin 3.9.x received error messages that their merge strategy value was not recognized. They had used 'default' as the value instead of 'DEFAULT' and the new code only accepted 'default'. This code now accepts all case variants of 'default'.

Checklist

  • I have read the CONTRIBUTING doc
  • I have referenced the Jira issue related to my changes in one or more commit messages
  • I have added tests that verify my changes
  • Unit tests pass locally with my changes
  • I have added documentation as necessary
  • No Javadoc warnings were introduced with my changes
  • No findbugs warnings were introduced with my changes
  • I have interactively tested my changes
  • Any dependent changes have been merged and published in upstream modules (like git-client-plugin)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Further comments

Relies on structs 1.18 for an API enhancement. Requires Jenkins 2.121.1 or newer. Backport of #699

@MarkEWaite MarkEWaite requested a review from jglick April 26, 2019 13:23
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine. I assume you will do a quick interactive sanity check that the bug as reported was reproducible in this branch before but not after this PR.

@jglick
Copy link
Member

jglick commented Apr 26, 2019

There are some test failures which do not seem to be in the base branch. They do not look obviously related to me—all involve an unexpected commit hash somewhere. Reproducible?

@MarkEWaite
Copy link
Contributor Author

Those test failures are intermittent and are also on the base branch. They seem to have become intermittent as a result of either the upgrade to Jenkins 2.121 or to the latest parent pom. When I configure surefire retries they will typically pass within the 5 retries that are configured. I've seen them intermittent on Ubuntu 16, Debian 10, FreeBSD 12, and Windows. In each of those cases, the surefire retries were enough to achieve a success by the fifth retry.

I'm prone to annotate those tests with @Ignore on the base branch for now and investigate later why they became intermittent with the transition from Jenkins 1.642 to Jenkins 2.121. I suspect it is related to timing assumptions, but I don't really have time to do the detailed investigation of those failures right now.

The API in JENKINS-44892 can be used to solve JENKINS-51638.

Backport of Jesse Glick's original implementation.
See jenkinsci#699
@MarkEWaite MarkEWaite merged commit 00b86c1 into jenkinsci:stable-3.10 Apr 27, 2019
@MarkEWaite MarkEWaite deleted the stable-3.10-JENKINS-51638 branch April 27, 2019 15:30
@MarkEWaite MarkEWaite added the bugfix Fixes a bug - used by Release Drafter label Jul 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes a bug - used by Release Drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants