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

[JENKINS-57694] Fix SpecificRevisionBuildChooser#getDisplayName() #760

Merged
merged 1 commit into from Sep 13, 2019

Conversation

darxriggs
Copy link
Contributor

@darxriggs darxriggs commented Sep 12, 2019

JENKINS-57694 - Fix SpecificRevisionBuildChooser#getDisplayName()

old description
All other BuildChoosers already have a Descriptor. This missing Descriptor was causing an exception when BuildChooser#getDisplayName() was called within GitSCM#compareRemoteRevisionWithImpl() to log its name.

The Descriptor was once already in place but then removed in commit 75e7880 because the SpecificRevisionBuildChooser is not meant to be selectable by a user in the UI.

The better way (because a Describable should have a Descriptor) to describe this fact is to overwrite isApplicable() to always return false.

new description
There may be BuildChoosers that don't have a Descriptor because they are not meant to be selectable by a user in the UI. Currently SpecificRevisionBuildChooser is such a BuildChooser.

This missing Descriptor was causing an exception when BuildChooser#getDisplayName() was called within GitSCM#compareRemoteRevisionWithImpl() to log its name.

The Descriptor was once already in place but then removed in commit 75e7880 because the SpecificRevisionBuildChooser is not meant to be selectable by a user in the UI.

Therefore the absence of a Descriptor has to be handled.

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 spotbugs 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)

@jglick
Copy link
Member

jglick commented Sep 12, 2019

Amends #709. I guess I am -0. See JIRA. Also IIRC there is another implementation that deliberately omits a Descriptor since it is intended only for programmatic construction.

…me()

There may be `BuildChooser`s that don't have a `Descriptor` because
they are not meant to be selectable by a user in the UI. Currently
`SpecificRevisionBuildChooser` is such a `BuildChooser`.

This missing `Descriptor` was causing an exception when
`BuildChooser#getDisplayName()` was called within
`GitSCM#compareRemoteRevisionWithImpl()` to log its name.

The `Descriptor` was once already in place but then removed in commit 75e7880
because the `SpecificRevisionBuildChooser` is not meant to be selectable by
a user in the UI.

Therefore the absence of a `Descriptor` has to be handled.
@darxriggs darxriggs marked this pull request as ready for review September 12, 2019 21:12
@MarkEWaite MarkEWaite merged commit 9659038 into jenkinsci:master Sep 13, 2019
@darxriggs darxriggs deleted the fix-missing-descriptor branch September 13, 2019 19:14
@MarkEWaite MarkEWaite added the bugfix Fixes a bug - used by Release Drafter label Sep 27, 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
3 participants