-
Notifications
You must be signed in to change notification settings - Fork 82
feat: 'e clean' for removing artifacts #680
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Will Anderson <andersonw@dropbox.com>
Co-authored-by: David Sanders <dsanders11@ucsbalum.com>
MarshallOfSound
left a comment
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 love the e clean name. Build Tools primary function is building electron, therefore e clean to me implies "clean my build" (other build tools have clean and it wipes the world).
This should probably be a subcommand of pr, but that doesn't feel right either. Honestly the original command probably shouldn't be pr rather something on ci.
E.g.
e ci download
e ci clear-downloads|
@MarshallOfSound Agreed on I think I was considering
If we come up with another use case for artifact files, we could consider moving to a top-level Regarding |
|
@MarshallOfSound do you have a preference on whether or not to merge first or to find a new name first? 🙂 |
|
@samuelmaddock I like the suggestion for |
1dbbd99 to
47acadb
Compare
Breaking this out as I think this warrants more discussion.
#679 introduces an
artifacts/directory for downloading pull request artifacts. To mitigate this directory growing indefinitely, I'm proposinge cleanande 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.
This presents the question of how often and when to display this warning.
A. Warn about stale files each time
e pr download-distis runB. Warn about stale files warning on
estartuplocalStorageAPI for caching stateMaybe this all too complex and we shouldn't do this at all? Open to feedback.