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

Support user regex filter for note trigger #923

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

the-sea
Copy link

@the-sea the-sea commented May 1, 2019

Support user regex filter when trigger type is note.If the username who wrote comment like "merge please" not match the user regex configured in job configuration, the note will not trigger build.

@atrifex
Copy link

atrifex commented Aug 1, 2019

Why hasn't this been reviewed and checked in? @omehegan can you please take a look.

@@ -67,8 +73,8 @@ protected CauseData retrieveCauseData(NoteHook hook) {
.withTargetProjectId(hook.getMergeRequest().getTargetProjectId())
.withBranch(hook.getMergeRequest().getSourceBranch())
.withSourceBranch(hook.getMergeRequest().getSourceBranch())
.withUserName(hook.getMergeRequest().getLastCommit().getAuthor().getName())
.withUserEmail(hook.getMergeRequest().getLastCommit().getAuthor().getEmail())
.withUserName(hook.getUser().getUsername())
Copy link

Choose a reason for hiding this comment

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

This is not backward compatible. People that are relying on this being the user name and email of committer will see bugs.

Might be useful to create new field that deals with the hook user.

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.

2 participants