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

Update Dashicons #2118

Merged
merged 4 commits into from
Aug 2, 2017
Merged

Update Dashicons #2118

merged 4 commits into from
Aug 2, 2017

Conversation

jasmussen
Copy link
Contributor

Don't merge this. There's a weird amount of red in this diff. I'm pushing as a branch so we can discuss how it regressed in the upstream dashicons repo, possibly as part of #1888.

BTW not trying to assign blame here or anything, it's probably somewhat trivial to fix, just making sure.

CC: @mtias, @EphoxJames (there appear to be some table icons) and @aduth

Don't merge this. There's a weird amount of red in this diff. I'm pushing as a branch so we can discuss how it regressed in the upstream dashicons repo, possibly as part of #1888.
@jasmussen jasmussen added the [Status] In Progress Tracking issues with work in progress label Aug 1, 2017
@codecov
Copy link

codecov bot commented Aug 1, 2017

Codecov Report

Merging #2118 into master will increase coverage by 1.66%.
The diff coverage is 7.14%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2118      +/-   ##
=========================================
+ Coverage   20.33%     22%   +1.66%     
=========================================
  Files         135     136       +1     
  Lines        4238    4558     +320     
  Branches      722     807      +85     
=========================================
+ Hits          862    1003     +141     
- Misses       2844    2967     +123     
- Partials      532     588      +56
Impacted Files Coverage Δ
components/dashicon/index.js 3.91% <7.14%> (+2.23%) ⬆️
blocks/library/verse/index.js 30.76% <0%> (-6.74%) ⬇️
blocks/library/list/index.js 7.33% <0%> (-0.88%) ⬇️
editor/modes/visual-editor/block.js 0% <0%> (ø) ⬆️
blocks/api/registration.js 100% <0%> (ø) ⬆️
editor/modes/visual-editor/index.js 0% <0%> (ø) ⬆️
editor/sidebar/post-status/index.js 0% <0%> (ø) ⬆️
editor/modes/visual-editor/block-list.js 0% <0%> (ø) ⬆️
editor/sidebar/post-author/index.js 0% <0%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ecd2ea...9808405. Read the comment docs.

@aduth
Copy link
Member

aduth commented Aug 2, 2017

Pushed 431ad49 to reintroduce case ordering (see WordPress/dashicons#222), which should reduce number of changes.

@aduth
Copy link
Member

aduth commented Aug 2, 2017

There was another missing upstream change. See also WordPress/dashicons#223, resolved here by 9808405. Order of the condition gets mixed up, but same effect.

@jasmussen
Copy link
Contributor Author

Thank you!

@jasmussen jasmussen merged commit c390037 into master Aug 2, 2017
@jasmussen jasmussen deleted the update/dashicons branch August 2, 2017 18:39
ceyhun pushed a commit that referenced this pull request Apr 22, 2020
…reen (#2118)

* update ref to master

* update RELEASE-NOTES.txt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants