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

[SPIKE] A more strict approach for finding versions in commit messages #397

Merged
merged 2 commits into from
Mar 21, 2015

Conversation

orjan
Copy link
Contributor

@orjan orjan commented Mar 18, 2015

It's hard to satisfy the tests without making the assumption that the relevant version is located at the first line. I don't the like the implementation but I think the added tests is correct?

References #385

* Applied a more strict approach to find version in merge commits
* Assume that the version is specified at the first commit message line

Fixed GitTools#385
@orjan
Copy link
Contributor Author

orjan commented Mar 20, 2015

I have a couple of issues with this implementation.

In order to support:

        [TestCase("Finish Release-0.12.0", true, "0.12.0")] //Support Syntevo SmartGit/Hg's Gitflow merge commit messages for finishing a 'Release' branch
        [TestCase("Finish 0.14.1", true, "0.14.1")] //Support Syntevo SmartGit/Hg's Gitflow merge commit messages for finishing a 'Hotfix' branch

and not library updates in merge commits with an aggregated commit history like:

Merge pull request #1 in FOO/bar from feature/ISSUE-1 to develop
* commit '38560a7eed06e8d3f3f1aaf091befcdf8bf50fea':
  Updated jQuery to v2.1.3

We'll need to make some assumptions:
orjan@db7beab#diff-ff51c8573866921bbeb1edf0998dc6f8R47
"^.*?(-|/|'|Finish )(?<PossibleVersions>\d+\.\d+\.\d+)"

  • The version is located at the first commit line
  • and starts with ', /, - or Finish

This makes the feature hard to understand for the users, why is my message picked up as a version or why not is it picked up. This makes the implementation error prone and fussy e.g. why if Finish a magical word?

Another option would be to make this feature optional and allow users to specify how to find the version in a commit message via configuration. But that will cause breaking changes.

Thoughts @JakeGinnivan?

@JakeGinnivan
Copy link
Contributor

This is fine, it passes all the tests so I am happy.

JakeGinnivan added a commit that referenced this pull request Mar 21, 2015
[SPIKE] A more strict approach for finding versions in commit messages
@JakeGinnivan JakeGinnivan merged commit 2fca958 into GitTools:master Mar 21, 2015
@orjan orjan deleted the merge-message-parser branch August 14, 2015 20:13
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.

None yet

2 participants