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

Multiline commit comments don't trigger notifications #716

Closed
mythgarr opened this issue Nov 18, 2014 · 2 comments

Comments

Projects
None yet
2 participants
@mythgarr
Copy link

commented Nov 18, 2014

We're using Perforce for version control and we use multi-line comments extensively for our check-ins. Using an alias of 'ajones' will trigger a notification for the following changelist:

Change 12345 by ajones@ajones_SOMECLIENTSPEC on 2014/11/18 10:00:00

Programmer Notes:

Affected files ...

... //depot/somepath/somefile.txt#2 edit

However, the following changelist will NOT be matched:

Change 12345 by ajones@ajones_SOMECLIENTSPEC on 2014/11/18 10:00:00

Programmer Notes:
* Some test notes

Affected files ...

... //depot/somepath/somefile.txt#2 edit

It looks like the culprit might be Matcher.java:83 - because the compiled pattern doesn't use DOTALL the trailing ".*" doesn't match. Testing with RegExPlanet.com confirms that the regex correctly matches both changelists if compiled with DOTALL.

@mythgarr

This comment has been minimized.

Copy link
Author

commented Nov 19, 2014

Looked again - the correct solution would probably be to replace

        if (pattern.matcher(comment).matches()) {

with

        if (pattern.matcher(comment).find()) {

mythgarr pushed a commit to mythgarr/gocd that referenced this issue Nov 21, 2014

arvindsv added a commit that referenced this issue Nov 25, 2014

Merge pull request #726 from mythgarr/master
Fix for Issue #716: The "Only if it contains my check-ins" option in notifications works now. It used to fail when there was a multi-line comment.

arvindsv added a commit to arvindsv/gocd that referenced this issue Nov 25, 2014

arvindsv added a commit that referenced this issue Nov 25, 2014

Merge pull request #735 from arvindsv/716_fix_build
#716 - Missing import statements. Fixing build.

@arikagoyal arikagoyal added this to the Release 14.4 milestone Dec 2, 2014

@arikagoyal

This comment has been minimized.

Copy link
Contributor

commented Dec 3, 2014

Works fine on 14.4.0(1252-6d6870f9df3547)

@arikagoyal arikagoyal closed this Dec 3, 2014

mdaliejaz added a commit to mdaliejaz/gocd that referenced this issue Dec 15, 2014

Revert "Fix for Issue gocd#716 - Matcher should now work correctly fo…
…r multi-line comments"

This reverts commit a86ba5e.

I will put the commit back once the CLA is signed by the sender of the PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.