Skip to content

Conversation

@recena
Copy link
Contributor

@recena recena commented Sep 14, 2017

JENKINS-43786

Downstream of jenkinsci/jenkins#2857

Screenshot

screenshot-2017-9-14 manage jenkins jenkins 1

@jenkinsci/code-reviewers


This change is Reviewable

@recena
Copy link
Contributor Author

recena commented Sep 19, 2017

@lanwen Thanks for your feedback. This PR has a drawback, it will require to bump the core version to the version where this PR jenkinsci/jenkins#2857 is merged.

@lanwen
Copy link
Member

lanwen commented Sep 19, 2017

@recena this is too huge change for this plugin...

@KostyaSha
Copy link
Member

@lanwen Thanks for your feedback. This PR has a drawback, it will require to bump the core version to the version where this PR jenkinsci/jenkins#2857 is merged.

Bad.

@recena
Copy link
Contributor Author

recena commented Sep 21, 2017

@lanwen Agree. You can apply these changes in midterm. This depends on changes in Jenkins. What I would like is to get those changes in core as soon as possible, and in this way, maintainers could adapt their plugins when they decide.

@recena
Copy link
Contributor Author

recena commented Sep 21, 2017

@KostyaSha Again I agree. I tried to share the pros / cons.

@lanwen
Copy link
Member

lanwen commented Sep 21, 2017

How would it look if I merge it now?

@lanwen
Copy link
Member

lanwen commented Oct 17, 2017

So it just changes the position of text in old versions
image

@lanwen lanwen merged commit 96242f8 into jenkinsci:master Oct 17, 2017
@recena
Copy link
Contributor Author

recena commented Oct 29, 2017

@lanwen I've worked on a more robust solution. You can see an example here jenkinsci/ssh-agents-plugin#70. Maintainers will be able to use the benefits of jenkinsci/jenkins#2857 without side-effects in previous versions.

You screenshot is showing a side-effect until the baseline of this plugin is updated.

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