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

Add a --remove-untracked option to the install command. #2172

Merged
merged 13 commits into from
May 1, 2020

Conversation

PetterS
Copy link
Contributor

@PetterS PetterS commented Mar 11, 2020

Fixes #648.

As discussed in the linked issue. @finswimmer seemed to approve this flag, @sdispater.

This is a feature that we are missing since moving away from pipenv. Removing old packages is part of reproducibility.

  • Added tests for changed code.
  • Updated documentation for changed code.

@PetterS PetterS force-pushed the install-keep-untracked branch from bbfc6c2 to 88bfa55 Compare March 11, 2020 08:55
@PetterS
Copy link
Contributor Author

PetterS commented Mar 17, 2020

Could anyone have a look at this? I am happy to finish this up with tests and documentation.

@finswimmer finswimmer requested a review from a team March 18, 2020 05:19
@PetterS PetterS force-pushed the install-keep-untracked branch from 4200711 to d717e0b Compare March 18, 2020 10:07
@PetterS PetterS changed the title Add a --keep-untracked option to the install command. Add a --remove-untracked option to the install command. Mar 18, 2020
@Mattwmaster58
Copy link

I'm not a poetry team member, but I took a quick look at this anyway, and I have to say you did a thorough job as far as I can tell 👍

@PetterS
Copy link
Contributor Author

PetterS commented Mar 25, 2020

Thanks Matt!

@stephsamson, do you think you could take a look?

@ulgens
Copy link

ulgens commented Mar 26, 2020

How does this handle packages like pip and setuptools which are not in lock file but should be kept in venv?

@PetterS
Copy link
Contributor Author

PetterS commented Mar 26, 2020

How does this handle packages like pip and setuptools which are not in lock file but should be kept in venv?

This is a great question and the answer is that, it did not, but currenty does!

kasteph
kasteph previously approved these changes Mar 26, 2020
Copy link
Member

@kasteph kasteph left a comment

Choose a reason for hiding this comment

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

This looks good to me but this feature was not in the original roadmap -- I'll defer to @sdispater on this one.

@PetterS
Copy link
Contributor Author

PetterS commented Apr 3, 2020

@sdispater did you have a chance to look at this? This is functionality that we really would like to have in poetry (we used pipenv clean before). From the upvotes and the issue, it seems we are not alone.

Thanks,
Petter

@PetterS
Copy link
Contributor Author

PetterS commented Apr 10, 2020

Is there something I could do here, besides waiting for a review? We'd love to have this in Poetry!

@abn abn requested a review from sdispater April 10, 2020 17:31
@abn abn added the area/cli Related to the command line label Apr 10, 2020
@ayroblu
Copy link

ayroblu commented Apr 28, 2020

Bumping this cause we're super interested in this

@PetterS
Copy link
Contributor Author

PetterS commented Apr 29, 2020

Bumping this cause we're super interested in this

Yes, the lack of this flag has been a pain point in my team since we switched from pipenv to poetry. People are rm -rf their venvs and installing from scratch.

How can we proceed here? @stephsamson @sdispater @finswimmer

@PetterS PetterS force-pushed the install-keep-untracked branch from 2bd022f to c4a2c62 Compare April 29, 2020 06:45
Copy link
Member

@kasteph kasteph left a comment

Choose a reason for hiding this comment

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

Thanks for pinging, sometimes things slip through the cracks and we forget. But thank you for the patience :)

One comment when I ran this by @sdispater: it can be improved by transferring most of the logic to the Solver class where it should reside and it should take care of the sensitive packages.

@PetterS
Copy link
Contributor Author

PetterS commented May 1, 2020

@stephsamson Done. Moved the logic to the solver.

We needed this option yesterday, when having issues with our cached CI environments. :-)

Wonder if we can get this into 1.1?

poetry/puzzle/solver.py Outdated Show resolved Hide resolved
@PetterS
Copy link
Contributor Author

PetterS commented May 1, 2020

OK, those type annotations did not work. I'll try to fix them.

Done

Copy link

github-actions bot commented Mar 1, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/cli Related to the command line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants