-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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 for git subdirectories #5172
Conversation
Linked to python-poetry/poetry-core#288 |
ddbbc9c
to
ca86f68
Compare
Pinging @sdispater @finswimmer |
Test requires |
@finswimmer Can this be included in the roadmap for 1.2? |
@ashnair1 Thanks for this! It appears there are tests failing that'll need to be resolved before a maintainer can merge. |
Pinging @finswimmer. Python tests passed. |
Pinging @finswimmer @abn. Could someone run the remaining workflows? |
|
Pinging @abn @finswimmer |
db20bd7
to
694a5eb
Compare
Some issues I’ve noticed: The
|
Seems #5648 has superseded this. |
Output on
|
Interesting. Thanks for the comments. I'll take a look. |
Any update on this? |
Pinging @neersighted @abn @finswimmer |
Hey all, this is still a priority to get merged/a blocker for 1.2 -- and it's still at or near the top of list of priorities. However, most of the core team has been indisposed due to non-Poetry circumstances for the last couple weeks. On my end my health and free time are both looking up in the near future, so I hope to get to this in the next week barring someone else striking first. Edit: Please don't repeatedly ping, posting on the issue will notify the interested maintainers. |
OK, I'm going to try to push this over the finish line. I tend not to squash all the commits so let's try to keep it clean. One thing in advance, which probably helps to understand the following proposals: In the best case, each commit changing a source file should add a test that requires that change (at least in my opinion 😉). I'd suggest proceeding as follows. At first please rebase on current master. Then, let's try to sort out the commits a bit:
I hope this is not too much to ask. If it is, I can also offer my support. |
So to recap, the commit order should be as follows:
Note: Text in bold will be the commit message. Is that correct? |
Correct. At first new tests that don't require any change in source and afterwards the rest in a sensible order. (A commit should not depend on a subsequent commit so that tests are succeeding all the time when stepping through the commits.) |
Done. Ready for review |
Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com>
Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com>
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 added some test coverage. LGTM now.
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. |
Pull Request Check List
Resolves: #755