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

Ignorable pusher #58

Open
wants to merge 47 commits into
base: master
Choose a base branch
from
Open

Ignorable pusher #58

wants to merge 47 commits into from

Conversation

aderigs
Copy link

@aderigs aderigs commented Jun 23, 2015

This defines a regular expression to which the name or email of an ignorable pusher is to be matched. Such a push will be ignored. Normally a preceding push of Jenkins him self could be prevented that way.

Review on Reviewable

@KostyaSha
Copy link
Member

This should work and be configured in GIT scm configuration.

@KostyaSha KostyaSha closed this Jun 23, 2015
@aderigs
Copy link
Author

aderigs commented Jun 23, 2015

How would you do this? The only possibility I have found would be something like "Polling ignores commits from certain users". But this would involve committers but not pushers.

@felfert
Copy link
Member

felfert commented Jun 23, 2015

@KostyaSha:

Wrong. You are confusing this with the existing featue of ignoring a commit-author. The pusher is not necessarily the committer!

@KostyaSha KostyaSha reopened this Jun 23, 2015
* @return the regular expression to which the name or email of an ignorable pusher is to be
* matched.
*/
String getIgnorablePusher();
Copy link
Member

Choose a reason for hiding this comment

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

this breaks backward compatibility

@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@igreenfield
Copy link

When this should be merged?

@lanwen
Copy link
Member

lanwen commented Feb 8, 2016

This will be never merged it current state, because of it provides only one case of filtering push (there is same for branches, committers, titles). But it still open because of its a good idea and not bad implementation. But we beed some more common implementation of filtering feature.

You can use hpi from the jenkins build to use this custom implementation (thanks @aderigs for rebasing)

@igreenfield
Copy link

What about using the Run Condition Plugin to support all kinds of filtering?

@KostyaSha
Copy link
Member

You can solve this issue by using intermediate decider job that can do additional verification and trigger real builds.
Pushers need to be implemented in more generic security API. API will be clear when you implement all possible gh triggers. I started this work in https://github.com/jenkinsci/github-integration-plugin .

@lanwen on other hand this 'hook -> gitscm poll' trigger wouldn't fit in any good APIs so probably such feature may be added to this trigger. But everything should be localised somewhere in code unique to this trigger.

@KostyaSha
Copy link
Member

Keeping in mind that gh utilises git polling log i can suggest try set exclusions using git configuration features (i.e. user exclusions). It wouldn't use gh names, but should work fine if people setting right names in git commits.

@igreenfield
Copy link

When I install the plugin with this change I don't see where to configure the new configuration!

@aderigs
Copy link
Author

aderigs commented Feb 9, 2016

Yes. The last version has introduced src/main/resources/com/cloudbees/jenkins/GitHubPushTrigger/config.groovy which hides src/main/resources/com/cloudbees/jenkins/GitHubPushTrigger/config.jelly. I think the clean way would be to migrate config.jelly into the new config.groovy. But I do not know how to do this.

@igreenfield
Copy link

the groovy file should be:

package com.cisco.jenkins.GitHubPushTrigger

import com.cisco.jenkins.GitHubPushTrigger

def f = namespace(lib.FormTagLib);

tr {
    td(colspan: 4) {
        f.entry(title: _("Ignorable Pusher"), field: "ignorablePusher") {
            f.textbox()
        }
    }
}

script(src: "${rootURL}${h.getResourcePath()}/plugin/github/js/warning.js")
script {
    text("""
InlineWarning.setup({ 
    id: 'gh-hooks-warn',
    url: ${descriptor.getCheckMethod('hookRegistered').toCheckUrl()}, 
    input: 'input[name="${GitHubPushTrigger.class.getName().replace(".", "-")}"]'
}).start();
""")
}

@codecov-io
Copy link

codecov-io commented Jul 24, 2016

Codecov Report

Merging #58 into master will increase coverage by 0.01%.
The diff coverage is 73.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #58      +/-   ##
==========================================
+ Coverage   75.07%   75.08%   +0.01%     
==========================================
  Files          64       64              
  Lines        1412     1425      +13     
  Branches      146      149       +3     
==========================================
+ Hits         1060     1070      +10     
+ Misses        307      306       -1     
- Partials       45       49       +4
Impacted Files Coverage Δ
...bhook/subscriber/DefaultPushGHEventSubscriber.java 82.14% <66.66%> (ø) ⬆️
.../java/com/cloudbees/jenkins/GitHubPushTrigger.java 61.24% <75%> (+0.89%) ⬆️
src/main/java/com/cloudbees/jenkins/Cleaner.java 22.22% <0%> (+5.55%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7927b4...b1d8812. Read the comment docs.

@KostyaSha
Copy link
Member

Please rebase and not merge

@ouaibsky
Copy link

@KostyaSha, it's why I did

git pull --rebase origin master

But don't know why, sounds not working, maybe missing some settings ?

@KostyaSha
Copy link
Member

That message to @aderigs who owns this PR

@kdawgwilk
Copy link

Does this support ignoring specific commit messages as well?

@aderigs
Copy link
Author

aderigs commented Aug 28, 2016

No. It ignores the pusher by name or email. If you want to ignore by specific commit messages you could use the git-plugin.

@kdawgwilk
Copy link

I have not been able to find a solution for this using the github folder plugin, which manages webhooks, and I believe uses this plugin. It automatically sets up the job and doesn't give you anywhere to configure the SCM that was setup. I just need a way to ignore webhook triggers if a specific commit message exists. I was hoping this would solve the issue :(

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.

9 participants