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

Don't ignore globs when skipping files for update #296

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RobertDeRose
Copy link

@RobertDeRose RobertDeRose commented Apr 2, 2024

The logic in _remove_paths checks if the path is a str or Path object and only if it is a str does it assume it contains a glob. Unfortunately, the _get_skip_paths method was wrapping all returned values in Path objects so globs were being ignored.

Fixes #295
Closes #258

@RobertDeRose RobertDeRose force-pushed the bugfix/glob-skips-are-ignored branch from 2310971 to 0528754 Compare April 2, 2024 20:32
The logic in `_remove_paths` checks if the path is a `str` or `Path`
object and only if it is a `str` does it assume it contains a glob.
Unfortunately, the `_get_skip_paths` method was wrapping all returned
values in `Path` objects so globs were being ignored.
@RobertDeRose RobertDeRose force-pushed the bugfix/glob-skips-are-ignored branch from 0528754 to d08b290 Compare April 2, 2024 20:39
Copy link

codecov bot commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.72%. Comparing base (1a5614f) to head (d08b290).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #296      +/-   ##
==========================================
+ Coverage   99.71%   99.72%   +0.01%     
==========================================
  Files          21       21              
  Lines        1050     1094      +44     
==========================================
+ Hits         1047     1091      +44     
  Misses          3        3              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RobertDeRose
Copy link
Author

@millsks You made the last commit 9 months ago. Is this project dead? This is the second PR to address this issue, I didn't see #258 which has been open since June 2023. I think my PR addresses the issue a little more closely to the current code style, with proper type hints, but one of these needs to be merged as it's a broken feature.

@millsks
Copy link
Contributor

millsks commented Apr 10, 2024

Hi Robert. I do not believe this project is dead. I just got side tracked by a move recently and trying to get everything situated again. I would say your PR would supercede mine if it takes unto account the latest code style.

@RobertDeRose
Copy link
Author

Hi Robert. I do not believe this project is dead. I just got side tracked by a move recently and trying to get everything situated again. I would say your PR would supercede mine if it takes unto account the latest code style.

Completely understood. Would appreciate a merge if this meets the maintainers satisfaction.
Glad to hear the project is not dead. Other than this issue, it's currently working pretty well for us. I would like to contribute some new features as time permits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Globs are ignored in skip list
2 participants