Feature/659 fix ip should not be matched as versions

[mkoertgen]

Fixes #659

[gep13]

I recently ran into this same issue on a repository at work, so I would be keen to get this merged in.

@JakeGinnivan what are your thoughts? The timeout issue that @mkoertgen mentioned here: #659 (comment) concerns me a little bit.

[JakeGinnivan]

I've never seen it timeout, but the logs have timing info in them, so should be easy to spot what I'd taking the time.

Other than that, pr looks good.

Sent from my Windows Phone

---

From: Gary Ewan Parkmailto:[email protected]
Sent: ‎4/‎10/‎2015 11:36 PM
To: GitTools/GitVersionmailto:[email protected]
Cc: Jake Ginnivanmailto:[email protected]
Subject: Re: [GitVersion] Feature/659 fix ip should not be matched as versions (#661)

I recently ran into this same issue on a repository at work, so I would be keen to get this merged in.

@JakeGinnivanhttps://github.com/JakeGinnivan what are your thoughts? The timeout issue that @mkoertgenhttps://github.com/mkoertgen mentioned here: #659 (comment)#659 (comment) concerns me a little bit.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/661#issuecomment-145359028.

[gep13]

@mkoertgen can you provide the logs for a GitVersion run where there is a timeout?

[mkoertgen]

Sorry, i did not mean to mention a timeout, just timing infos with GitVersion /output buildserver.

In fact, i do not believe the adjusted RegExp to become a bottleneck. However, i have not much experience with timing in GitVersion.

For my issue #659 gitversion looked about 330 commits back for the ip message. And it took about 0.17 seconds. So right now performance seems to be quite good.

I did not check timing again with the fix included, though.

I think, the PR can be merged in. If someone gets a performance a hit i'll take care of it. word!

[JakeGinnivan]

This will not be a performance bottleneck in comparison to other things. Negative lookbehinds can be slow, but the strings we have are small and compared to some of the git operations we do, it will be negligible.

[gep13]

👍 I will try to give the new version a try this morning on the repo that was giving me problems, and will report back as well

[gep13]

👍 this is now working on the internal repo that was exhibiting the same behaviour 😸