build-tools
build-tools copied to clipboard
feat: 'e clean' for removing artifacts
Breaking this out as I think this warrants more discussion.
https://github.com/electron/build-tools/pull/679 introduces an artifacts/ directory for downloading pull request artifacts. To mitigate this directory growing indefinitely, I'm proposing e clean and e clean --stale.
e cleanwill remove theartifacts/directorye clean --staleremoves only top-level stale files inartifacts/(older than one month)
So folks are aware of these commands existence—and that stale files exist—we can display a warning.
$ e pr download-dist 44653
Stale artifact(s) found:
pr_44653_darwin_arm64
Run 'e clean --stale' to cleanup artifacts.
This presents the question of how often and when to display this warning.
A. Warn about stale files each time e pr download-dist is run
- Check for stale files and log warning
B. Warn about stale files warning on e startup
- Every time build-tools is invoked, we check for stale files and report on them
- To prevent spam, only warn once a week.
localStorageAPI for caching state
Maybe this all too complex and we shouldn't do this at all? Open to feedback.
@MarshallOfSound Agreed on e clean not feeling quite right. It was mostly chosen to avoid analysis paralysis.
I think I was considering artifacts/ to be something broader than only PR downloads. However, maybe I should limit the scope initially. How about this:
e pr download-distas it exists- Logs notice about stale dist artifacts
e pr clean-distremoves all dist artifactse pr clean-dist --staleremoves all stale dist artifacts
If we come up with another use case for artifact files, we could consider moving to a top-level clean command of some sort.
Regarding pr vs ci. I guess my mind goes to "I want to download this PR's artifacts" rather than "I want to download this CI's artifacts"
@MarshallOfSound do you have a preference on whether or not to merge first or to find a new name first? :slightly_smiling_face:
@samuelmaddock I like the suggestion for e pr download-dist, e pr clean-dist and e pr clean-dist --stale. I do think there's some debating we could do about e pr vs e ci, but I personally wouldn't block over it. If you want to implement that, I think we could merge this and iterate later if we wanted to 😄