Bump pathspec to 0.10.3 #8767
Bump pathspec to 0.10.3 #8767
Conversation
karajan1001
requested review from
skshetry and
dtrifiro
and removed request for
skshetry
karajan1001
force-pushed
the
fix8217
branch
2 times, most recently
from
3fea844 to
743209e
Compare
Codecov ReportBase: 93.64% // Head: 93.64% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #8767 +/- ##
==========================================
- Coverage 93.64% 93.64% -0.01%
==========================================
Files 456 456
Lines 36122 36128 +6
Branches 5227 5228 +1
==========================================
+ Hits 33827 33832 +5
Misses 1799 1799
- Partials 496 497 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
karajan1001
force-pushed
the
fix8217
branch
3 times, most recently
from
198053f to
67d1d20
Compare
| GitWildMatchPattern.pattern_to_regex(pattern_info.patterns) | ||
| for pattern_info in pattern_list | ||
| ] | ||
| self.regex_pattern_list = [] |
There was a problem hiding this comment.
Looks good. Do you have benchmarks for this?
There was a problem hiding this comment.
No, I hadn't, The PR only did some name field jobs and didn't touch the matching logic.
| @@ -87,7 +87,7 @@ def mock_dvcignore(dvcignore_path, patterns): | |||
| ("to_ignore.txt", ["/*.txt"], True), | |||
| (os.path.join("path", "to_ignore.txt"), ["/*.txt"], False), | |||
| (os.path.join("data", "file.txt"), ["data/*"], True), | |||
| (os.path.join("data", "subdir", "file.txt"), ["data/*"], False), | |||
| # (os.path.join("data", "subdir", "file.txt"), ["data/*"], False), | |||
There was a problem hiding this comment.
It's a newly introduced bug by pathspec. need to give more comments here.
There was a problem hiding this comment.
Is this a core use case? Do we need to maybe stick with an older version and report/fix this bug in pathspec? Is there a pathspec issue for it?
There was a problem hiding this comment.
Yes, we need to report to them.
There was a problem hiding this comment.
@karajan1001, can we workaround this? I don't think we should merge this with this bug otherwise.
There was a problem hiding this comment.
@karajan1001 accidental merge or do you plan on fixing it in a followup?
There was a problem hiding this comment.
The current behavior is the same as git.
g check-ignore dir/subdir/a -v [ins]
.gitignore:2:dir/* dir/subdir/a
We should change it to
(os.path.join("data", "subdir", "file.txt"), ["data/*"], True),
fix: iterative#8217 1. Replace group name to avoid conflict. 2. Disable failed tests. 3. Bump pathspec to 0.10.3
|
@karajan1001 There are a few unaddressed points above. Accidental merge? |
Original from iterative#8767 (comment)
fix: #8217
pathspecto 0.10.3Wait for
scmrepoto update to iterative/scmrepo#163❗ I have followed the Contributing to DVC checklist.
📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
Thank you for the contribution - we'll try to review it as soon as possible. 🙏