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

Fix pathlib issues #219

Merged
merged 5 commits into from
Aug 10, 2023
Merged

Fix pathlib issues #219

merged 5 commits into from
Aug 10, 2023

Conversation

mbarkhau
Copy link
Owner

@mbarkhau mbarkhau commented Aug 8, 2023

Fixes #204 #218

@SRFU-NN
Copy link

SRFU-NN commented Aug 9, 2023

This fixes the problem for me.

A rough test that fails if the problem is there, and passes when it is fixed:

from bumpver import config

def test_capitalisation():
    filename_list = [
        'test/temp/CAPITALIZED.txt',
        'test/temp/uncapitalized.txt',    
    ]
    for filename in filename_list:
        with open(filename,"w") as f:
            f.write("This is a test file for testing capitalization")
    raw_patterns = {filename: filename for filename in filename_list}
    for file_pattern in config._iter_glob_expanded_file_patterns(raw_patterns):
        filtered_pattern = file_pattern[0]
        filtered_pattern = filtered_pattern.replace("\\","/")
        unfiltered_pattern = file_pattern[1]
        assert filtered_pattern == unfiltered_pattern

Note that the test leaves the folder test\temp with contents behind, so it isn't production ready.

Also note that when I try to run bumpver update --patch in the repo, it complains that it can't find the patterns in the files src/bumpver/hooks.py and src/bumpver/pathlib.py, because they lack the copyright notice at the start that contains the pattern src/bumpver/*.py = Copyright (c) 2018-YYYY. I am not sure what the "correct" action is when using wildcards in filenames, and only some of the matching files contain the pattern, but it is worth to consider.

@SRFU-NN
Copy link

SRFU-NN commented Aug 10, 2023

If you need a more formal review from me, I need access to make them.

@mbarkhau
Copy link
Owner Author

No, that's fine. I'll look over your test, add it if I can and then I think we're good to go for a release.

@mbarkhau mbarkhau merged commit ea758fe into master Aug 10, 2023
4 checks passed
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.

Commiting issues with case sensitivity in filenames
2 participants