Skip to content

Comments

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

Merged
kasteph merged 13 commits intopython-poetry:developfrom
PetterS:install-keep-untracked
May 1, 2020
Merged

Add a --remove-untracked option to the install command.#2172
kasteph merged 13 commits intopython-poetry:developfrom
PetterS:install-keep-untracked

Conversation

@PetterS
Copy link
Contributor

@PetterS PetterS commented Mar 11, 2020 •
edited
Loading

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

Mattwmaster58 commented Mar 25, 2020

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 •
edited
Loading

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
View reviewed changes
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 •
edited
Loading

@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 •
edited
Loading

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 72b8fab to 2bd022f Compare April 29, 2020 06:45
@PetterS PetterS force-pushed the install-keep-untracked branch from 2bd022f to c4a2c62 Compare April 29, 2020 06:45
kasteph
kasteph reviewed Apr 29, 2020
View reviewed changes
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 •
edited
Loading

@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?

kasteph
kasteph reviewed May 1, 2020
View reviewed changes
@PetterS
Copy link
Contributor Author

PetterS commented May 1, 2020 •
edited
Loading

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

Done

@kasteph kasteph merged commit 7049bd5 into python-poetry:develop May 1, 2020
@kasteph kasteph mentioned this pull request May 1, 2020
Closed
2 tasks
@seansfkelley seansfkelley mentioned this pull request May 1, 2020
Closed
@sdispater sdispater mentioned this pull request Jun 9, 2020
Closed
2 tasks
@sdispater sdispater mentioned this pull request Jun 26, 2020
Merged
@mjlbach mjlbach mentioned this pull request Sep 1, 2020
Closed
2 tasks
@finswimmer finswimmer mentioned this pull request Sep 29, 2020
Closed
2 tasks
@seansfkelley seansfkelley mentioned this pull request Jun 21, 2021
Closed
3 tasks
@github-actions
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.

Reviewers

@kasteph kasteph kasteph approved these changes

@sdispater sdispater Awaiting requested review from sdispater

+1 more reviewer

@snejus snejus snejus left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@sdispater sdispater

Labels

area/cli Related to the command line

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@PetterS @Mattwmaster58 @ulgens @ayroblu @kasteph @snejus @abn @sdispater