Skip to content

Conversation

@mattjbray
Copy link

#3 matches email addresses in the ident field. Also allow email addresses in the auth field.

@untergeek
Copy link
Contributor

Please add a test (or better yet, tests) to https://github.com/logstash-plugins/logstash-patterns-core/tree/master/spec/patterns to guarantee that usage will work as expected and not break other patterns.

@mattjbray mattjbray force-pushed the patch-1 branch 2 times, most recently from 975fc87 to 9df7cb6 Compare September 22, 2015 15:50
logstash-plugins#3 matches email addresses in the `ident` field.  Also allow email addresses in the `auth` field.
@mattjbray
Copy link
Author

@untergeek I've added a couple of tests for this change and for the change in #3.

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

@purbon
Copy link

purbon commented Nov 5, 2015

please jenkins, test this.

Copy link

Choose a reason for hiding this comment

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

good additions to the test suites, I wonder if you could also add some that match the "former" USER pattern too? like this we'll be sure there are no regressions introduced.

@purbon
Copy link

purbon commented Nov 5, 2015

@mattjbray thanks a lot for your contribution, I just left you a very small comment. As soon as we're good with it, I'm very welcome to merge this. Looking forward to it.

@mattjbray
Copy link
Author

@purbon I've added a test for matching the USER pattern in both fields.

@purbon
Copy link

purbon commented Nov 6, 2015

@mattjbray awesome, good job!

@purbon
Copy link

purbon commented Nov 6, 2015

please jenkins, test this.

@jsvd jsvd closed this Sep 15, 2016
@jsvd jsvd reopened this Sep 15, 2016
@jsvd
Copy link
Member

jsvd commented Sep 15, 2016

this has been fixed in #142, thanks for the contribution!

@jsvd jsvd closed this Sep 15, 2016
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.

5 participants