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.

Sign up for GitHub

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

Jump to bottom

support glob keyword in configfile #150

Merged
merged 2 commits into from Aug 24, 2020
Merged

support glob keyword in configfile #150

merged 2 commits into from Aug 24, 2020

Conversation

balrok
Copy link
Contributor

@balrok balrok commented Jul 20, 2020

This implements #64

@ekohl ekohl linked an issue Jul 21, 2020 that may be closed by this pull request
globbing in config file parsing, like [bumpversion:file:*.py] #64
Closed
ekohl
ekohl reviewed Jul 21, 2020
View reviewed changes
Copy link
Collaborator

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a good implementation. Some minor suggestions inline.

bumpversion/cli.py Show resolved Hide resolved
bumpversion/cli.py Outdated

if section_type.get("file") == "glob":
for filename_glob in glob.glob(filename):
files.append(ConfiguredFile(filename_glob, VersionConfig(**section_config)))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps VersionConfig(**section_config) should be stored in a variable before the if section and used in both branches.

@balrok
Copy link
Contributor Author

balrok commented Jul 21, 2020

thanks for your comments in both PR :-)
having the VersionConfig created outside of the loop will also reuse the object between the globbed-files

@ekohl
Copy link
Collaborator

ekohl commented Jul 21, 2020

having the VersionConfig created outside of the loop will also reuse the object between the globbed-files

Yes. I think that's good because it only sets variables in the constructor, not at runtime. This saves some resources.

ekohl
ekohl approved these changes Jul 21, 2020
View reviewed changes
Copy link
Collaborator

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Since I made some suggestions, I'd appreciate if another maintainer also has a look before merging.

@balrok balrok force-pushed the glob-keyword-in-configfile branch from 78780b4 to a96ebf4 Compare July 23, 2020 17:48
@balrok
Copy link
Contributor Author

balrok commented Aug 7, 2020

just one small fix, I was not aware that python-glob does not interpret "**" by default as recursive.. one has to set the recursive-flag in this function before.. there is now also a test for this

ekohl
ekohl approved these changes Aug 24, 2020
View reviewed changes
Copy link
Collaborator

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to merge this since there was no objection from another maintainer and it looks good to me.

@ekohl ekohl merged commit 0a38097 into c4urself:master Aug 24, 2020
@ekohl
Copy link
Collaborator

ekohl commented Aug 24, 2020

Thanks @balrok!

@florisla florisla added this to the v1.0.1 milestone Jan 12, 2021
@dependabot dependabot bot mentioned this pull request Mar 15, 2021
Merged
@lowell80
Copy link
Contributor

lowell80 commented Apr 1, 2021

Awesome feature, thanks @balrok! You just saved me a bunch of time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekohl ekohl ekohl approved these changes

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
v1.0.1
Development

Successfully merging this pull request may close these issues.

globbing in config file parsing, like [bumpversion:file:*.py]
4 participants
@balrok @ekohl @lowell80 @florisla