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: modify regex to enforce key name of "version" #76

Closed

Conversation

nkirkes
Copy link

@nkirkes nkirkes commented May 28, 2014

Currently, regex will match anything with "version" in the key name. When tracking more than one version scheme, such as "apiVersion", and if that additional scheme is the first occurrence the regex finds, the bump will be applied to that key:value.

Change to regex removes the ? modifier for quotes which A) enforces proper JSON format and B) restricts key name for the bump target to "version" (still case insensitive).

@nkirkes
Copy link
Author

nkirkes commented May 28, 2014

I should note, in my case, this was relevant in a config.json file I wanted to bump along with the package.json. In my config.json, I'm tracking several version numbers for different APIs/uses, but have one "version" that I wanted to bump. grunt-bump was updating only the first occurrence the regex hit, which was an incorrect target, potentially causing other versioning problems.

Maybe a better way to mitigate this is by allowing a custom defined key name in the bump options, defaulting to "version" if one is not defined.

@eddiemonge
Copy link
Collaborator

I like your second suggestion about the custom defined key.

  • Sample json files to test this change against
  • Commit message doesn't follow guidelines

@eddiemonge
Copy link
Collaborator

ping @nkirkes

@eddiemonge
Copy link
Collaborator

I wonder if it would be better to only require JSON files and then parse it as JSON and get the root version key that way.

@nkirkes
Copy link
Author

nkirkes commented Jun 17, 2014

That would make it easier, yes. Are you still thinking that the root version key would be defined as an option (defaulting to "version")?

@eddiemonge
Copy link
Collaborator

possibly yes. Im not sure about specifying a key to use though

@nkirkes
Copy link
Author

nkirkes commented Jun 17, 2014

Hmm.. Let me think on that. If you don't mind, I'll take a crack at implementing this.

@skobkin
Copy link

skobkin commented Jan 7, 2015

Had the same problem: #113.

@eddiemonge
Copy link
Collaborator

Closing as this isn't the best fix. See #119 for more info

@eddiemonge eddiemonge closed this Jan 28, 2015
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.

3 participants