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

feat: cleanup flag #1086

Merged
merged 4 commits into from
Aug 27, 2024
Merged

Conversation

CodeWithEmad
Copy link
Member

This will introduce -c --clean command, to make sure we have a new environment on save.

Close #967

@kdmccormick
Copy link
Collaborator

Thanks @CodeWithEmad ! I can take a look in a couple weeks after the conference and my vacation.

@CodeWithEmad
Copy link
Member Author

Thank you, @kdmccormick. There's no rush for it. Enjoy your vacation.

@CodeWithEmad CodeWithEmad force-pushed the feat/cleanup-flag branch 2 times, most recently from 27dafaa to 9399845 Compare July 9, 2024 08:15
tutor/env.py Outdated Show resolved Hide resolved
tutor/interactive.py Outdated Show resolved Hide resolved
@CodeWithEmad CodeWithEmad force-pushed the feat/cleanup-flag branch 3 times, most recently from 49dcea4 to c2b1c19 Compare July 18, 2024 13:14
@kdmccormick kdmccormick self-requested a review July 22, 2024 18:00
tutor/interactive.py Outdated Show resolved Hide resolved
tutor/commands/k8s.py Show resolved Hide resolved
tutor/commands/config.py Outdated Show resolved Hide resolved
tutor/commands/compose.py Outdated Show resolved Hide resolved
tutor/commands/k8s.py Outdated Show resolved Hide resolved
tutor/interactive.py Outdated Show resolved Hide resolved
@kdmccormick kdmccormick removed their request for review August 9, 2024 17:04
@CodeWithEmad CodeWithEmad force-pushed the feat/cleanup-flag branch 4 times, most recently from 6faad48 to 70b6239 Compare August 11, 2024 09:55
@@ -0,0 +1 @@
- 💥[Feature] Enhance your workflow with the new `-c` or `--clean` option for the `tutor config save` command! This feature allows you to clean your environment before each save, ensuring that all files and directories within the `env/` folder are deleted, providing you with a fresh environment each time. (by @CodeWithEmad)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not a breaking change. The feature is only accessible behind a new argument. we can remove 💥 from the changelog.

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1. Also, instead of calling on all users to enhance their workflow with this new flag, could you explain why a user would want to use it? Reading #967, my understanding is this most users will not need to use -c; rather, only plugin developers or other advanced users will need it, as they are the ones who will need to clean files out of their environment.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not a breaking change.

Oops. My bad.

could you explain why a user would want to use it?

How about this?

  • [Feature] Added -c or --clean option to tutor config save: For plugin developers and advanced users, this option cleans the env/ folder before saving, ensuring a fresh environment for testing and development.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perfect 🚀

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@kdmccormick kdmccormick removed their request for review August 19, 2024 18:37
Copy link
Collaborator

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

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

Pending Dawoud's and my most recent comments, this looks good to me!

This will add a `-c` `--clean` flag to the save command and ensure that the env directory is deleted if it exists.

Close overhangio#967
Before this, the "Interactive platform configuration" message was shown even if -I flag was set for tutor dev|local launch.
@kdmccormick kdmccormick merged commit 46b4016 into overhangio:master Aug 27, 2024
2 checks passed
@kdmccormick
Copy link
Collaborator

Thanks @CodeWithEmad !!

@CodeWithEmad CodeWithEmad deleted the feat/cleanup-flag branch August 28, 2024 07:34
regisb added a commit that referenced this pull request Oct 31, 2024
The introduction of the `-c/--clean` option caused the deletion prompt
to be displayed for every call to `tutor config save --interactive`.
This is not the desired behaviour, as decided here:
#1086 (comment)

With this change, the prompt is only displayed when running: `tutor
config save --interactive --clean`. The environment is still deleted on
`tutor config save --clean`, but without prompt.

We refactored the code with hooks, which simplifies the signature of the
interactive prompt function.
regisb added a commit that referenced this pull request Nov 1, 2024
The introduction of the `-c/--clean` option caused the deletion prompt
to be displayed for every call to `tutor config save --interactive`.
This is not the desired behaviour, as decided here:
#1086 (comment)

With this change, the prompt is only displayed when running: `tutor
config save --interactive --clean`. The environment is still deleted on
`tutor config save --clean`, but without prompt.

We refactored the code with hooks, which simplifies the signature of the
interactive prompt function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Clean root/env directory before saving config
4 participants