-
Notifications
You must be signed in to change notification settings - Fork 108
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
Add plugin banner and icon assets #231
Conversation
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.
you'll also want to add the .wordpress-org
directory to the .gitattributes
file
Also, please ensure that you prop @rwlands and any others from the source issue so that we're properly crediting folks for this effort |
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.
My only concern is the SVG icon which is almost 8 MB
in size, it seems like the SVG is including nonrequired stuff. It would be great if we can have a smaller size created out of geometric shapes instead of actual images.
Ah right, that makes sense from a GitHub perspective. Just double-checking, will the 10up deploy action still consider the directory for the asset deployment though if it's "ignored" via
Absolutely, however right now we have no process at all for any props for this plugin. This would be mostly relevant for eventual WP core merges for sure, but how about during plugin development? How do other feature plugins handle that? If there's something we can do here I'd say let's open an issue to put the necessary guidelines / tooling in place.
Good catch! Can you potentially look into that @rwlands? Indeed the SVG file is extremely large at the moment. |
from the action readme, which also matches my experience with the action on 10up's plugins:
Ah right, yeah I suppose we should get something in place soon before it becomes unwieldy to determine who to prop if (🤞🏼 when 🤞🏼) the plugin merges to core. 10up tends to utilize a CREDITS.md file (example) to keep track of those folks across versions as well as proping them on changelog items. I could help go back through things already merged and gather those props into the changelog and/or credits file, but curious which approach (or both) you would prefer to help track things ahead of a core merge? |
Sounds good, updated!
A credits file would help, however I think ours may need to be more granular than the one you've linked. There won't be a moment where "the plugin merges to core", since this is multiple WP core performance features in different stages of development, and they would merge individually when they're ready respectively. So to best support this workflow we would probably need some sort of credits file that is broken down by the different modules in this plugin - which is still possible, but a bit more involved. Would you mind creating an issue so we can explore this further? |
@felixarntz to further clarify my understanding before opening the issue, perhaps a credits file within the sub-directories in https://github.com/WordPress/performance/tree/trunk/modules would suffice for your concern? Otherwise this PR looks ✅ to me. |
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.
Hooray, beautiful branding!
That's an alternative option, yes. I think we should discuss that in the eventual issue, whether it's preferable to have
|
@mitogh I just removed the |
Hi there, I can look into the svg size issue, that's certainly very overweight! |
Sounds good @felixarntz looks like a new SVG was added tho in case we want to add those as well. Thank you @rwlands for getting these SVG icons for us, great work 🎉 |
Thanks so much for fixing the SVG @rwlands! This is now good to be merged 🎉 |
Summary
Fixes #144
Relevant technical choices
.wordpress-org
folder will automatically deployed with every release tag via our GitHub workflow.Checklist
[Focus]
orInfrastructure
label.[Type]
label.no milestone
label.