-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Move/deprecate conda_env
modules into conda
#13168
Conversation
fad6b4f
to
10d2599
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some broad stroke observations:
- all new deprecations are for 24.3 and 24.9 (not 23.9 and 24.3)
- all functions, classes, and modules marked as deprecated from before should not move to
conda.env
- need to combine
conda.env.cli
withconda.cli
(common
is probably the easiest to do, can wait withmain
for now)
9afa89b
to
1227e5b
Compare
7b34e64
to
03ce776
Compare
I think this looks good so far. Before I give my approval, I want to clone this locally and poke around at it though. Please wait to merge until I have done this 🙏. |
Got a chance to poke around at it more... One thought I had while I was looking through it is why we aren't we taking the chance to combine the I'm okay if that's too much to pack into a single pull request, but I would like to know the plans for eventually getting rid our usage of that file. It would be really nice for us to to have a single |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good!
Now to eliminate unnecessary code paths (e.g. calling conda env
still invokes the conda-env
entrypoint which in turn imports from conda_env
etc.).
Thoughts on moving tests/conda_env
to tests/env
and moving all of the new import tests into a new tests/test_conda_env.py
?
This is a good idea, implemented in latest commit 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small nit that would be dedicated to help with #13291 (comment)
9f5a083
to
961728e
Compare
961728e
to
68c4881
Compare
5ffef00
Minor merge conflicts due to #13433 that I just fixed. The PR has obviously been approved a few times now. |
This PR addresses the following epic:
conda_env
intoconda
#11633To do:
/conda/tests/conda_env/test_cli.py
to testconda_env
function imports work as expected (before moving anything around)conda_env
modules intoconda
Checklist - did you ...
news
directory (using the template) for the next release's release notes?