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

make the clean task remove directories instead of emptying them #11

Merged
merged 3 commits into from
May 21, 2020

Conversation

ryanatkn
Copy link
Owner

This changes the gro clean task to remove directories entirely instead of just emptying them. Gro's current heuristic for performing a compilation when running a task currently just looks for the presence of a build directory.

Previously, running gro clean && gro <task> as a user would not trigger compilation, but with this change it does.

Although the heuristic leads to problems in some circumstances, it's a fine solution for now. The fix is to just run gro clean.

changelog.md Outdated
@@ -1,5 +1,9 @@
# Gro changelog

## unreleased
Copy link
Contributor

Choose a reason for hiding this comment

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

do you know what the this release version this will be in? Any reason to not just preemptively set it here?

Copy link
Owner Author

@ryanatkn ryanatkn May 21, 2020

Choose a reason for hiding this comment

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

ya it'll be 0.1.6, I did that as a workflow thing copying another project I saw, but my reasoning was flimsy - basically didn't want to confuse that 0.1.6 could already be released. But that's not really an issue for us. Certainly easier to just put the version that it should be and not worry about changing it in the future. I'll just put the target version going forward unless you have any thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm, there's got to be some sort of best practice for updating the version in the changelog and releasing at the same time but it escapes me at the moment. But I think the confusion is better than forgetting to do another PR to update the version on release.

Copy link
Owner Author

@ryanatkn ryanatkn May 21, 2020

Choose a reason for hiding this comment

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

Good point, I agree about the confusion is the better of the tradeoffs. Sounds like something we should/could eventually automate, probably trivial to do in preversion, maybe with a simple task.

Copy link
Contributor

@greatbacon greatbacon left a comment

Choose a reason for hiding this comment

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

And remember, ABC : Always Build Clean!

@ryanatkn ryanatkn merged commit 7e49402 into master May 21, 2020
@ryanatkn ryanatkn deleted the remove-on-clean branch May 21, 2020 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants