-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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-73047] Fix weather icons #9227
Conversation
Yay, your first pull request towards Jenkins core was created successfully! Thank you so much! |
ok |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code change looks good as far as I understand it.
@SNanda8895 Could you confirm testing in all supported browsers looked OK with and without zoom applied (ideally by providing screenshots for each combination)? Given this issue occurred only in one browser and with zoom applied, we'd like to prevent causing regressions in other browsers. These are the supported browsers.
Additionally, please provide a suggested changelog entry. See the changelog styleguide for guidance and previous entries for examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the screenshots.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/label ready-for-merge
This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.
Thanks!
Congratulations on getting your very first Jenkins core pull request merged 🎉🥳 |
Fix weather icons (cherry picked from commit ee22f8b)
See JENKINS-73047.
Testing done
I tested it locally ,everything worked fine.
Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Desired reviewers
@Kevin-CB
@daniel-beck
Before the changes are marked as
ready-for-merge
:Maintainer checklist