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

fix: explicit source dependency is not satisfied by direct origin #7973

Merged
merged 2 commits into from
May 26, 2023

Conversation

ralbertazzi
Copy link
Contributor

@ralbertazzi ralbertazzi commented May 20, 2023

Resolves: #7722

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

cc @dimbleby

@ralbertazzi ralbertazzi force-pushed the fix/mix-direct-source branch 3 times, most recently from 8d072c3 to 298c906 Compare May 20, 2023 17:42
@dimbleby
Copy link
Contributor

So far as the code change goes I'm basically reviewing my own suggestion here. Looks fine to me, but this is not an independent review.

I think I would have expected to write a testcase here that more closely follows the actual problem as laid out in the issue report. Probably something in test_installer.py that sets things up and runs poetry install and checks that the expected thing happens.

I always find those testcases unpleasant to write, which is partly what put me off doing anything with this one in the first place. If you can convince yourself - and someone with the commit bit - that what you have is sufficient, then your life will be easier.

@ralbertazzi
Copy link
Contributor Author

I added a test, LMK if that's what you were looking for ;)

@ralbertazzi ralbertazzi force-pushed the fix/mix-direct-source branch 2 times, most recently from b38ed8f to 4b0a796 Compare May 24, 2023 13:00
@radoering
Copy link
Member

I added a solver test. There is some overlap with the installer test, but since it's not (only) an installer issue but first of all a solver issue (i.e. the package with the explicit source is not only not installed but not locked), it made sense to me.

Now, we should have more than sufficient test coverage. 😀

@radoering radoering added the impact/docs Contains or requires documentation changes label May 26, 2023
@github-actions
Copy link

github-actions bot commented May 26, 2023

Deploy preview for website ready!

✅ Preview
https://website-1pp39wxs6-python-poetry.vercel.app

Built with commit 42364f1.
This pull request is being automatically deployed with vercel-action

@radoering radoering added impact/backport Requires backport to stable branch backport/1.5 labels May 26, 2023
@radoering radoering merged commit c77ffbd into python-poetry:master May 26, 2023
poetry-bot bot pushed a commit that referenced this pull request May 26, 2023
)

Co-authored-by: David Hotham <[email protected]>
Co-authored-by: Randy Döring <[email protected]>
(cherry picked from commit c77ffbd)
radoering pushed a commit that referenced this pull request May 26, 2023
)

Co-authored-by: David Hotham <[email protected]>
Co-authored-by: Randy Döring <[email protected]>
(cherry picked from commit c77ffbd)
@ralbertazzi ralbertazzi deleted the fix/mix-direct-source branch May 29, 2023 16:33
Copy link

github-actions bot commented Mar 3, 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 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
impact/backport Requires backport to stable branch impact/docs Contains or requires documentation changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple environment markers doesn't work
3 participants