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

chore: split up make clean target #1029

Merged
merged 3 commits into from
Sep 17, 2024
Merged

chore: split up make clean target #1029

merged 3 commits into from
Sep 17, 2024

Conversation

gphorvath
Copy link

@gphorvath gphorvath commented Sep 12, 2024

Description

Splitting up some of the make targets for cleaning up artifacts, cache, etc. This will help when the intent is only delete build artifacts or models, without also hitting .env or ruff_cache.

BREAKING CHANGES

None

CHANGES

  • Replaced make clean with an include for mk-clean.mk
  • Split make clean functionality into clean-artifacts, clean-cache, clean-env, clean-models, and clean-logs
  • Added make clean-all to replicate previous functionality.

Related Issue

None, just an problem spotted in the course of rebuilding.

Checklist before merging

Copy link

netlify bot commented Sep 12, 2024

Deploy Preview for leapfrogai-docs canceled.

Name Link
🔨 Latest commit 2a012b4
🔍 Latest deploy log https://app.netlify.com/sites/leapfrogai-docs/deploys/66e490e17450020008c9a1cd

@gphorvath gphorvath added tech-debt Not a feature, but still necessary chore and removed tech-debt Not a feature, but still necessary labels Sep 12, 2024
@gphorvath gphorvath self-assigned this Sep 12, 2024
@gphorvath gphorvath marked this pull request as ready for review September 12, 2024 18:12
@gphorvath gphorvath requested a review from a team as a code owner September 12, 2024 18:12
justinthelaw
justinthelaw previously approved these changes Sep 12, 2024
Copy link
Contributor

@justinthelaw justinthelaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nitpick that technically doesn't need to be here - can we include a UDS clean too?

This works on Debian based distros, but the temp files may be located elsewhere in Mac and Windows. The temp files do take up a TON of space if your OS clean-up job isn't often or thorough.

uds zarf tools clear-cache && rm -rf ~/.uds-cache && rm -rf /tmp/zarf-*

@gphorvath
Copy link
Author

gphorvath commented Sep 13, 2024

One nitpick that technically doesn't need to be here - can we include a UDS clean too?

This works on Debian based distros, but the temp files may be located elsewhere in Mac and Windows. The temp files do take up a TON of space if your OS clean-up job isn't often or thorough.

uds zarf tools clear-cache && rm -rf ~/.uds-cache && rm -rf /tmp/zarf-*

@justinthelaw I don't disagree that this is a valuable alias. My only concern with adding another make target to this PR is that it would exceed the scope of LeapfrogAI by hitting system level zarf/uds caches. By limiting our cleanup to files within this directory we avoid that. We have uds-clean documented as an alias in DEVELOPMENT.md, do you think that is sufficient to satisfy your need?

PS: I need a rereview anyway due to a typo.

@gphorvath gphorvath merged commit 2d9cff2 into main Sep 17, 2024
24 checks passed
@gphorvath gphorvath deleted the make-refactor branch September 17, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants