Skip to content

Inline ChangeRequestSCMHead2 interface into ChangeRequestSCMHead#444

Open
das7pad wants to merge 3 commits into
jenkinsci:masterfrom
das7pad:change-request-scm-head-cleanup
Open

Inline ChangeRequestSCMHead2 interface into ChangeRequestSCMHead#444
das7pad wants to merge 3 commits into
jenkinsci:masterfrom
das7pad:change-request-scm-head-cleanup

Conversation

@das7pad

@das7pad das7pad commented May 25, 2026

Copy link
Copy Markdown

This PR is cleaning up the ChangeRequestSCMHead/ChangeRequestSCMHead2 interface workaround that was needed back before Java 8.

Testing done

I've reviewed the usages of ChangeRequestSCMHead in the jenkinsci org and did not spot any instances that would not satisfy the ChangeRequestSCMHead2 requirement. So we could remove the default implementation as well.

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests that demonstrate the feature works or the issue is fixed

@das7pad das7pad requested a review from a team as a code owner May 25, 2026 08:50

@jglick jglick left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There are usages right in this plugin that would need to be adapted, namely

return !(head instanceof ChangeRequestSCMHead2)
|| ((ChangeRequestSCMHead2) head).getCheckoutStrategy() != ChangeRequestCheckoutStrategy.HEAD;
and then I think a downstream PR would need to be prepared for https://github.com/jenkinsci/branch-api-plugin/blob/e73a56d397882c63ae06483ba81742c2e39d25d5/src/main/java/jenkins/branch/BranchNameContributor.java#L73-L75

* @since 2.2.0
*/
// TODO once Java 8 is baseline move method to ChangeRequestSCMHead with default return value,
// TODO deprecate this interface and add @Restricted(NoExternalUse.class) (retain empty interface for binary compat)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should it not be deprecated?

@NonNull
String getOriginName();
}
@Restricted(NoExternalUse.class)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can it be DoNotUse? That way it blocks usages in this plugin too?

Suggested change
@Restricted(NoExternalUse.class)
@Restricted(DoNotUse.class)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Restricted is inappropriate. It is simply @Deprecated.

@das7pad

das7pad commented Jun 16, 2026

Copy link
Copy Markdown
Author

There are usages right in this plugin that would need to be adapted, namely

Thanks for the pointers, that's addressed in d8d6da5. I've also switch the annotation to @deprecated in d2ec56f.

and then I think a downstream PR would need to be prepared for https://github.com/jenkinsci/branch-api-plugin/blob/e73a56d397882c63ae06483ba81742c2e39d25d5/src/main/java/jenkins/branch/BranchNameContributor.java#L73-L75

👍 I've opened a PR over there:

@das7pad das7pad requested a review from jglick June 16, 2026 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants