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

added nox -e clean #60

Merged
merged 20 commits into from
Sep 8, 2024
Merged

added nox -e clean #60

merged 20 commits into from
Sep 8, 2024

Conversation

DavideTisi
Copy link
Collaborator

added nox -e clean to clean all the temporary files in examples.

followed @ceriottm discussion this is a proposal for implementing the clean function in nox.
it uses git ls-files --other example to check all the files not git tracked in the example dir

Copy link

github-actions bot commented May 5, 2024

Here is the build version of the cookbook in this pull request: documentation.zip, you can view it locally by unzipping documentation.zip and open the index.html with your favorite browser.

noxfile.py Outdated Show resolved Hide resolved
noxfile.py Outdated Show resolved Hide resolved
noxfile.py Show resolved Hide resolved
noxfile.py Outdated Show resolved Hide resolved
noxfile.py Outdated Show resolved Hide resolved
@ceriottm
Copy link
Contributor

ceriottm commented May 8, 2024

With @Luthaf we were discussing of how we should call this target. clean-build seemed a good descriptive one. I'd also add docs/build, and .nox if you haven't added those already.

@DavideTisi
Copy link
Collaborator Author

I thought we did not want to add .nox

@ceriottm
Copy link
Contributor

ceriottm commented Aug 23, 2024

OK, I went through this and as far as I'm concerned, I'm happy. I separated clean_build from clean_examples, as I think people may want to keep experiments in examples sometimes, but need to clean the build. Maybe this approach gives us more flexibility to keep stuff we don't want to remove for some reason.

@DavideTisi
Copy link
Collaborator Author

for me it is fine

@DavideTisi
Copy link
Collaborator Author

@ceriottm do we want to merge this?
@Luthaf @PicoCentauri what do you think ?

@ceriottm
Copy link
Contributor

ceriottm commented Sep 5, 2024

Lgtm, I think I cleaned it up a while ago

noxfile.py Outdated
Comment on lines 81 to 82
# Get the list of ignored files
# Get the list of all not tracked files
Copy link
Contributor

Choose a reason for hiding this comment

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

comment seems duplicated

noxfile.py Outdated Show resolved Hide resolved
noxfile.py Outdated
"""Remove temporary files and building folders"""

# remove building folders and the nox cache
for i in ["docs/src/examples/", "docs/build", ".nox"]:
Copy link
Contributor

Choose a reason for hiding this comment

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

why .nox? this will remove itself when running =/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ceriottm I think you added that piece

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Even without michele comment I kind of agree that removing folders like .nox is the point of this function

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, I see this more as a way to clean temporary files created by the example, not to clean the virtualenvs. Also rm -rf .nox is trivial compared to what nox -e clean does, so leaving them separate is not too much of a burden on the end user.

Copy link
Contributor

Choose a reason for hiding this comment

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

So, the rationale for this whole PR is to have a setup similar to the clean_python script of other projects, wiping all build and caches. Upon checking, it's true that all of those don't wipe .tox so even though I added it in the first place, I now agree that for consistency we should not (or we should do this consistently everywhere). Removing it for now (and merging)

noxfile.py Outdated Show resolved Hide resolved
DavideTisi and others added 2 commits September 5, 2024 16:11
Co-authored-by: Guillaume Fraux <[email protected]>
Co-authored-by: Guillaume Fraux <[email protected]>
@ceriottm ceriottm merged commit f06e7ea into main Sep 8, 2024
12 checks passed
@ceriottm ceriottm deleted the feat/clean branch September 8, 2024 11:06
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.

4 participants