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.

Sign up for GitHub

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

Jump to bottom

Update the Plugin Manager Updates View with more Info about incompatible dependencies and change CSS classes #4299

Merged
merged 7 commits into from
Oct 31, 2019
Merged

Update the Plugin Manager Updates View with more Info about incompatible dependencies and change CSS classes #4299

merged 7 commits into from
Oct 31, 2019

Conversation

andipabst
Copy link
Contributor

This is a hacktoberfest contribution.

Proposed changelog entries

  • Add information about incompatible dependencies to the updates view in the plugin manager
  • Change the classes for the warnings in the updates view to use 'alert-danger'

Before:
Bildschirmfoto am 2019-10-16 um 16 26 26

After: Bildschirmfoto am 2019-10-16 um 16 22 29

@daniel-beck
Copy link
Member

CC @oleg-nenashev

@daniel-beck daniel-beck self-requested a review October 16, 2019 14:41
@oleg-nenashev oleg-nenashev added hacktoberfest rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted web-ui The PR includes WebUI changes which may need special expertise labels Oct 16, 2019
@oleg-nenashev oleg-nenashev self-requested a review October 16, 2019 14:44
oleg-nenashev
oleg-nenashev reviewed Oct 16, 2019
View reviewed changes
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Thanks! Just few minor comments

zbynek
zbynek reviewed Oct 16, 2019
View reviewed changes
Copy link
Contributor

@zbynek zbynek left a comment

Choose a reason for hiding this comment

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

The new look is nice!

core/src/main/java/hudson/model/UpdateCenter.java Outdated
@@ -970,11 +970,25 @@ private static String getCategoryDisplayName(String category) {

public List<Plugin> getUpdates() {
Map<String,Plugin> pluginMap = new LinkedHashMap<>();
Map<String, List<Plugin>> incompatiblePluginMap = new LinkedHashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

If we would index this map with Plugin instead of plugin name, we could avoid lookup in pluginMap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please explain this further to me?
If incompatiblePluginMap would be indexed with Plugin, how would I iterate through the pluginMap later and add the incompatible plugins there?

@oleg-nenashev oleg-nenashev added the needs-more-reviews Complex change, which would benefit from more eyes label Oct 17, 2019
oleg-nenashev
oleg-nenashev approved these changes Oct 22, 2019
View reviewed changes
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Looks good from the behavior/compatibility standpoint. I am not that concerned about performance for this page, but it definitely needs discussion with @zbynek to be finished before merging

@oleg-nenashev oleg-nenashev requested a review from a team October 22, 2019 22:11
zbynek
zbynek approved these changes Oct 23, 2019
View reviewed changes
Copy link
Contributor

@zbynek zbynek left a comment

Choose a reason for hiding this comment

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

Thanks for checking the suggestions!

@fcojfernandez fcojfernandez added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Oct 24, 2019
alecharp
alecharp approved these changes Oct 24, 2019
View reviewed changes
Copy link
Member

@alecharp alecharp left a comment

Choose a reason for hiding this comment

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

I find the new UI less aggressive and still more noticeable than the old one. Thanks!

@fcojfernandez
Copy link
Member

thanks @andipabst ! I've added the ready for merge label.

@oleg-nenashev oleg-nenashev merged commit e5ba39e into jenkinsci:master Oct 31, 2019
@oleg-nenashev
Copy link
Member

Thanks a lot for your contributions @andipabst ! It is much appreciated, and we invite you to keep contributing to the project. 👍

@andipabst andipabst deleted the plugin-manager-updates-view branch November 1, 2019 09:08
@andipabst
Copy link
Contributor Author

Thank you all for your help in this pull request. I would very much like to keep contributing!

@daniel-beck daniel-beck mentioned this pull request Mar 16, 2020
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alecharp alecharp alecharp approved these changes

@zbynek zbynek zbynek approved these changes

@oleg-nenashev oleg-nenashev oleg-nenashev approved these changes

@fcojfernandez fcojfernandez fcojfernandez approved these changes

@daniel-beck daniel-beck Awaiting requested review from daniel-beck

@batmat batmat Awaiting requested review from batmat

@varyvol varyvol Awaiting requested review from varyvol

@raul-arabaolaza raul-arabaolaza Awaiting requested review from raul-arabaolaza

Assignees
No one assigned
Labels
needs-more-reviews Complex change, which would benefit from more eyes ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted web-ui The PR includes WebUI changes which may need special expertise
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@andipabst @daniel-beck @fcojfernandez @oleg-nenashev @alecharp @zbynek @varyvol