-
-
Notifications
You must be signed in to change notification settings - Fork 227
New git file finder that support symlinks. #247
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
Conversation
it seems this breaks the expectations of some tests which is ok, |
eb33bfb
to
3b83b3a
Compare
Make the test compliant with file finder spec (although integration.find_files is not spec compliant yet, because it defaults to '.' with path is empty)
3b83b3a
to
c42f7e2
Compare
No need to be sorry about anything. The work behing done here is far from easy, with huge expectations from the world. It's green now. |
setuptools_scm/git_file_finder.py
Outdated
@@ -0,0 +1,67 @@ | |||
# -*- coding: utf-8 -*- | |||
# Copyright © 2018 ACSONE SA/NV | |||
# License LGPLv3 (http://www.gnu.org/licenses/lgpl-3.0-standalone.html) |
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.
i just noted the licence, is it acceptable for you to go to MIT?
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.
yep, sorry, copy/paste error, I remove that
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.
generally well done, this is how i like a change 😀 👍
please take a look at the licensing detail
wow, that's quick ^^ |
Thanks. Is appveyor running tests on windows? I don't find the link. |
i need to check to see why its not there |
it seems a configuration issue is preventing the ci run, it'll take a bit to investigate |
indeed, it got strangely messed up in a token cleanup and appveyor never told details until i recreated the project |
1e7f475
to
5dcae32
Compare
5dcae32
to
b548b50
Compare
it seems like windows is giving and giving 😞 |
226e5b8
to
2fbdcff
Compare
@RonnyPfannschmidt a bit of trial and error indeed :) I think I nailed it now. Basically it boils down to using normcase everywhere when looking up in the git files list. I chose to preserve case and separators in the finder output. The last two failing tests also fail on master. |
a8ccefb
to
2fbdcff
Compare
thanks for finalizing the details, i'll take a look at the rest |
4ad0fbe
to
2fbdcff
Compare
ok, as far as i understand this is merge-able and the tests that dont work in windows are my fault due to developing while windows ci was accidentally off please ack then i'll merge |
yes, this PR is ready to merge |
great job, thanks for jumping all the hops to iron that one out |
Closes #245