-
Notifications
You must be signed in to change notification settings - Fork 9
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
VACMS-18269 Remove unused assets #2242
base: main
Are you sure you want to change the base?
Conversation
5b660a3
to
9cef993
Compare
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.
LGTM! Nice clean up
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.
Is there any other way to verify that these files aren't being used besides a code search? Do we have access to analytics or some other kind of reporting that could show traffic requests to these files?
src/site/assets/img/sprite.svg
Outdated
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.
I don't think this sprite sheet should be removed. It's added for the va-icon
web component. See this readme for a list of places this file should be active: https://github.com/department-of-veterans-affairs/dst-uswds-compile/blob/main/README.md
9cef993
to
a862e2b
Compare
@jamigibbs Good callout about the sprite file; I've re-added it. I did find one instance (and there may be a few more) where content-build is referring to vets-website styles for an image that should be in the content-build repo. We have a ticket to straighten out those cross-repo styles (here) that we haven't started yet. We'll definitely need to straighten that out before coming back to this. Other than that, and any design system references, there shouldn't be files out in the ether referring to assets in content-build (and if they are, they need to point to a different image bucket). But I'll have another look and see if I can find any analytics to that effect. Thanks for the review! |
This PR is stale because it has been open 120 days with no activity. It will receive a stale label every day for 14 days before being closed unless it receives a comment or the stale label is removed. |
Bumping to keep this from getting closed |
DO NOT MERGE
Doing some extra research to make sure images that aren't explicitly in the content-build code (i.e. cross-referenced in vets-website styles or used by the design system) are not removed. This is a WIP
Summary
Removed unused assets from the content-build assets (and assets-removed) folders.
Related issue(s)
Testing done
Searched the content-build repo for usage of any of these files. Zero results.