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

Reframe use of --isolated in tool run #5470

Merged
merged 1 commit into from
Jul 30, 2024
Merged

Reframe use of --isolated in tool run #5470

merged 1 commit into from
Jul 30, 2024

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Jul 26, 2024

Summary

This PR gets rid of the global --isolated flag (which serves a bunch of independent responsibilities right now) on uv tool run in favor of a dedicated --isolated flag, which tells uv to avoid re-using an existing tool environment for this invocation. We'll add the same thing to uv run, to avoid using the base project environment.

This will become a bit clearer in #5466, when we deprecate the --isolated flag on the preview APIs.

@charliermarsh charliermarsh added preview Experimental behavior cli Related to the command line interface labels Jul 26, 2024
@zanieb
Copy link
Member

zanieb commented Jul 30, 2024

What's the idea behind "isolate" vs "isolated"? What tense do we want to use in our CLI?

@charliermarsh
Copy link
Member Author

I suppose we could use --isolated. I wanted to have a hard break with the deprecated flag, otherwise in a vacuum I would've just used --isolated. Let me skim our CLI and see if we have examples either way...

@zanieb
Copy link
Member

zanieb commented Jul 30, 2024

Yeah I worry about using a different name than we would have just for the deprecation :/ it makes some sense pragmatically but I am also genuinely curious which tense we should use in general.

@charliermarsh
Copy link
Member Author

It looks like we tend to use imperative: --refresh, --reinstall, --annotate, --generate-hashes

@charliermarsh
Copy link
Member Author

On the other hand, we have --locked.

@charliermarsh
Copy link
Member Author

charliermarsh commented Jul 30, 2024

Although --locked isn't an action (it's not telling you to lock something), it's a description... Whereas --isolate is telling uv to add isolation. I don't know if I'm smart enough to describe the distinction accurately.

@zanieb
Copy link
Member

zanieb commented Jul 30, 2024

I think I feel the gist of what you're getting at.

I think if we consider --locked (and similar) as "descriptions of how to act" rather than "doing another action" (as would be implied by --lock), I think --isolated actually makes more sense because we're doing the same action just in a different way?

@charliermarsh
Copy link
Member Author

Yeah that's a good description. Ok, should we just reuse --isolated here? It's not a huge problem since --isolated already meant this in these contexts. It just makes the deprecation and warnings more difficult to implement...

@zanieb
Copy link
Member

zanieb commented Jul 30, 2024

I think we would use it if not for the existing flag; and, since it's the same as before and mostly in preview, I think we should go for it. Sorry it adds complexity 😬 hopefully the deprecation is short-lived? I'm also okay with no-deprecation in the preview commands.

Base automatically changed from charlie/pin to main July 30, 2024 17:52
@charliermarsh
Copy link
Member Author

I'm not 100% sure how to make it work since we'll now have --isolated as both a global and an argument on a dedicated command (assuming we want it to show up in help). I'll try though.

@charliermarsh
Copy link
Member Author

Oh interesting, Clap lets commands override global arguments. Ok, that's easier...

@charliermarsh charliermarsh changed the title Remove use of --isolated in tool run Reframe use of --isolated in tool run Jul 30, 2024
@charliermarsh charliermarsh enabled auto-merge (squash) July 30, 2024 19:05
@charliermarsh charliermarsh merged commit ff3bcbb into main Jul 30, 2024
55 of 56 checks passed
@charliermarsh charliermarsh deleted the charlie/isolate branch July 30, 2024 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command line interface preview Experimental behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants