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

VACMS-18269 Remove unused assets #2242

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

randimays
Copy link
Contributor

@randimays randimays commented Aug 16, 2024

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.
Screenshot 2024-08-16 at 3 52 10 PM
Screenshot 2024-08-16 at 3 51 45 PM
Screenshot 2024-08-16 at 3 51 30 PM
Screenshot 2024-08-16 at 3 51 15 PM
Screenshot 2024-08-16 at 3 51 04 PM
Screenshot 2024-08-16 at 3 50 55 PM
Screenshot 2024-08-16 at 3 50 40 PM
Screenshot 2024-08-16 at 3 50 12 PM
Screenshot 2024-08-16 at 3 49 40 PM
Screenshot 2024-08-16 at 3 49 12 PM
Screenshot 2024-08-16 at 3 49 01 PM
Screenshot 2024-08-16 at 3 48 54 PM
Screenshot 2024-08-16 at 3 48 48 PM
Screenshot 2024-08-16 at 3 47 57 PM
Screenshot 2024-08-16 at 3 47 42 PM
Screenshot 2024-08-16 at 3 47 20 PM
Screenshot 2024-08-16 at 3 47 08 PM
Screenshot 2024-08-16 at 3 46 34 PM
Screenshot 2024-08-16 at 3 46 18 PM
Screenshot 2024-08-16 at 3 46 12 PM
Screenshot 2024-08-16 at 3 46 01 PM
Screenshot 2024-08-16 at 3 45 53 PM
Screenshot 2024-08-16 at 3 45 07 PM
Screenshot 2024-08-16 at 3 43 47 PM
Screenshot 2024-08-16 at 3 42 52 PM
Screenshot 2024-08-16 at 3 42 04 PM
Screenshot 2024-08-16 at 3 41 47 PM
Screenshot 2024-08-16 at 3 40 50 PM
Screenshot 2024-08-16 at 3 40 01 PM
Screenshot 2024-08-16 at 3 26 59 PM

@va-vfs-bot va-vfs-bot temporarily deployed to master/main/18269-remove-unused-assets August 16, 2024 20:27 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/18269-remove-unused-assets August 16, 2024 21:10 Inactive
@randimays randimays marked this pull request as ready for review August 19, 2024 14:46
@randimays randimays requested review from a team as code owners August 19, 2024 14:46
Copy link
Contributor

@chriskim2311 chriskim2311 left a 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

Copy link
Contributor

@jamigibbs jamigibbs left a 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?

Copy link
Contributor

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

@randimays
Copy link
Contributor Author

@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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants