-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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(core): relax pathspec upper bound version limitation #9373
Conversation
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @rzjfr |
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Thanks for opening @rzjfr Could you do the following?
|
Thanks for the review @dbeatty10, I did both 👍 While merging main back to the PR I realised this commit which was arguing for keeping the upper bound as it's in major-version-0, so I just upgraded 0.12 to 0.13 while I was resolving the conflict. What do you think? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9373 +/- ##
==========================================
- Coverage 88.06% 88.02% -0.05%
==========================================
Files 178 178
Lines 22439 22439
==========================================
- Hits 19762 19751 -11
- Misses 2677 2688 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
@rzjfr Thanks for contributing! This looks good to me!
Do you mind push a commit to get the code quality check passing? Then I can merge.
Hi @ChenyuLInx , Sorry for that, It must have been failing when I created my branch and I didn't realize. |
Any chance this will be merged soon? |
+1 We will need this to unblock our process. Thanks very much. |
resolves #9372
Problem
Starting from dbt-core 1.2.6 this dependency has been added: pathspec>=0.9,<0.12 which conflicts with latest version of pathspec (0.12.1). I can't find any reason specifically for <0.12. It seems to be consequence of dependebot upgrading more restrictive pathspec~=0.9 here and then later here
Solution
I can't find any reason to restrict pathspec <0.12. I couldn't find any major change that can break anything related to this package in pathspec change log. Here I just removed the upper bound. If we want to be for some reason very conservative about this particular package we can change it either to <0.13 or <1.
Checklist