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-73805] #9788

Closed
wants to merge 7 commits into from
Closed

[JENKINS-73805] #9788

wants to merge 7 commits into from

Conversation

scherler
Copy link
Contributor

@scherler scherler commented Sep 26, 2024

See JENKINS-73805.

Testing done

Proposed changelog entries

  • human-readable text

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

@mention

Before the changes are marked as ready-for-merge:

Maintainer checklist

Copy link
Member

@daniel-beck daniel-beck left a comment

Choose a reason for hiding this comment

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

This looks like it would be far more straightforward by making AboutJenkins StaplerOverridable. Why did you choose this approach?

*
* @since 2.478
*/
public class AboutPageDecorator extends Descriptor<AboutPageDecorator> implements ExtensionPoint, Describable<AboutPageDecorator> {
Copy link
Member

Choose a reason for hiding this comment

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

Why Descriptor/Describable? Seems entirely unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For lack of knowledge, I used #8462 for inspiration I will look into StaplerOverridable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@daniel-beck I have refactored the code, does that look better?

Signed-off-by: Thorsten Scherler <[email protected]>
Signed-off-by: Thorsten Scherler <[email protected]>
@scherler scherler closed this Oct 1, 2024
@scherler scherler deleted the JENKINS-73805 branch October 1, 2024 06:14
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.

2 participants