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

Remove unused image assets #5777

Merged
merged 5 commits into from
Nov 12, 2021
Merged

Conversation

janfaracik
Copy link
Contributor

@janfaracik janfaracik commented Oct 3, 2021

Proposed changelog entries

  • Remove superfluous user interface elements, including images, mask-icon, and unnecessary resize handles.

What's changed?

  • Removed a fair few unused assets from Jenkins/combined assets where possible to cut down on space
  • Removed mask-icon - https://www.leereamsnyder.com/blog/favicons-in-2021 - Jenkins now has a lovely colourful icon on Safari ❤️
  • Textareas no longer have superfluous resize handles on browsers that support resize (anything that's not IE11)

Screenshots

Old
image

New
image


Old
image

New
image

Maintainer checklist

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

  • There are at least 2 approvals for the pull request and no outstanding requests for change
  • Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
  • Changelog entries in the PR title and/or Proposed changelog entries are correct
  • Proper changelog labels are set so that the changelog can be generated automatically
  • If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
  • If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

@oleg-nenashev oleg-nenashev requested a review from a team October 5, 2021 17:01
@jglick
Copy link
Member

jglick commented Oct 6, 2021

Have you searched plugin sources to make sure there are no users of the small icons proposed for deletion?

@janfaracik
Copy link
Contributor Author

Have you searched plugin sources to make sure there are no users of the small icons proposed for deletion?

Had a scan through and there's a few that are in use by extremely old plugins (eg ten years old) that haven't had any updates. The only plugin that I found that has recent updates is https://github.com/jenkinsci/release-plugin - it's currently using the atom.gif icon, I'd be happy to make an MR for it to use the new RSS icon currently used by Jenkins

@timja
Copy link
Member

timja commented Oct 6, 2021

tbh can't even remember why I touched that plugin, I assume to do with tables to divs but I can't tell now =/. I can merge and release it by the looks of it though.

Copy link
Member

@timja timja 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 just needs release plugin sorted from what I can see

@janfaracik
Copy link
Contributor Author

janfaracik commented Nov 9, 2021

Looks good just needs release plugin sorted from what I can see

Just opened a PR for that - jenkinsci/release-plugin#36 :)

@timja timja requested review from jglick, a team, fqueiruga and uhafner November 9, 2021 17:26
@jglick jglick removed their request for review November 9, 2021 17:42
@timja timja added 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 Nov 9, 2021
Copy link
Contributor

@fqueiruga fqueiruga left a comment

Choose a reason for hiding this comment

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

Nice. off topic, I think I should create a PR to remove the experimental header we introduced... 2 years ago 😅

Copy link
Contributor

@fqueiruga fqueiruga left a comment

Choose a reason for hiding this comment

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

Nice. off topic, I think I should create a PR to remove the experimental header we introduced... 2 years ago 😅

@timja
Copy link
Member

timja commented Nov 10, 2021

Nice. off topic, I think I should create a PR to remove the experimental header we introduced... 2 years ago 😅

Yeah probably, not worth keeping that tech debt around since it was never enabled by default


This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

@timja timja added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Nov 10, 2021
@timja timja merged commit 5b26013 into jenkinsci:master Nov 12, 2021
@janfaracik janfaracik deleted the remove-unused-assets branch January 5, 2022 15:08
@daniel-beck
Copy link
Member

Caused JENKINS-69319.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

6 participants