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.

Sign up for GitHub

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

Jump to bottom

Fix #2597 by checking that the directory exists #2602

Merged
merged 2 commits into from
Jun 30, 2020
Merged

Fix #2597 by checking that the directory exists #2602

merged 2 commits into from
Jun 30, 2020

Conversation

PetterS
Copy link
Contributor

@PetterS PetterS commented Jun 29, 2020

Resolves: #2597

  • Added tests for changed code.

@PetterS
Copy link
Contributor Author

PetterS commented Jun 29, 2020

@abn

@PetterS
Copy link
Contributor Author

PetterS commented Jun 29, 2020

Will fix the Python 2.7 errors if this is reasonable.

abn
abn requested changes Jun 29, 2020
View reviewed changes
Copy link
Member

@abn abn left a comment

Choose a reason for hiding this comment

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

We can potentially make use of the site.addpackage() method here. See suggestions.

poetry/repositories/installed_repository.py Outdated Show resolved Hide resolved
poetry/repositories/installed_repository.py Outdated Show resolved Hide resolved
@PetterS
Copy link
Contributor Author

PetterS commented Jun 30, 2020

Did the suggested changes. I don't understand the code well enough to write really good tests here, I'm afraid. The test currently mocks site.addpackage.

abn
abn requested changes Jun 30, 2020
View reviewed changes
Copy link
Member

@abn abn left a comment

Choose a reason for hiding this comment

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

See comments, you should not need to change existing tests since the functionality should remain the same. Additionally, you will need to add couple of new fixtures in the istalled repository fixture such that the following cases are also evaluated;

  1. Multiple lines in .pth file.
  2. Executable in .pth file.

tests/repositories/test_installed_repository.py Outdated Show resolved Hide resolved
@PetterS
Copy link
Contributor Author

PetterS commented Jun 30, 2020

OK, I reverted the test file so you can see the error.

abn
abn requested changes Jun 30, 2020
View reviewed changes
poetry/repositories/installed_repository.py Outdated Show resolved Hide resolved
abn
abn requested changes Jun 30, 2020
View reviewed changes
poetry/repositories/installed_repository.py Show resolved Hide resolved
poetry/repositories/installed_repository.py Outdated Show resolved Hide resolved
@abn abn self-requested a review June 30, 2020 13:00
@abn
Copy link
Member

abn commented Jun 30, 2020

@PetterS I have applied a few changes. The changes themselves work; but need to rework for compatibility and add test case for executable .pth files.

abn
abn reviewed Jun 30, 2020
View reviewed changes
poetry/repositories/installed_repository.py Outdated Show resolved Hide resolved
poetry/repositories/installed_repository.py Outdated Show resolved Hide resolved
@abn abn force-pushed the fix_2597 branch 2 times, most recently from 60b2a8a to 2978cbe Compare June 30, 2020 22:07
@abn abn self-requested a review June 30, 2020 22:15
@abn abn merged commit f6b0680 into python-poetry:develop Jun 30, 2020
@PetterS
Copy link
Contributor Author

PetterS commented Jul 1, 2020

Thanks for fixing this!

@abn abn linked an issue Jul 1, 2020 that may be closed by this pull request
Poetry 1.1.0a2 includes python code in the version string #2597
Closed
@sdispater sdispater mentioned this pull request Jul 10, 2020
Merged
@github-actions 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

@abn abn abn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Poetry 1.1.0a2 includes python code in the version string
2 participants
@PetterS @abn