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

Permit use of Forbidden files without File Paths #251

Merged
merged 1 commit into from Sep 30, 2015

Conversation

Projects
None yet
3 participants
@scoheb
Copy link
Contributor

commented Sep 29, 2015

When a job is configured to be triggered with the use of a Forbidden path,
this path is only inspected if the configuration ALSO contains a File path.

This patch corrects this requirement.

[JENKINS-29663]

@jenkinsadmin

This comment has been minimized.

Copy link
Member

commented Sep 30, 2015

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

@@ -818,8 +818,7 @@ public boolean equals(Object obj) {
&& p.isInteresting(changeBasedEvent.getChange().getProject(),
changeBasedEvent.getChange().getBranch(),
changeBasedEvent.getChange().getTopic())) {
if (isFileTriggerEnabled() && p.getFilePaths() != null
&& p.getFilePaths().size() > 0) {
if (isFileTriggerEnabled()) {

This comment has been minimized.

Copy link
@rsandell

rsandell Sep 30, 2015

Member

This scares me a little, because it means that there will be a callback to query files for each change-based event that enters Jenkins instead of if there is interest in files. (Indicated by the extra ssh commands you needed to mock in the tests).
I think you can get the same effect if you just also check p.getForbiddenFilePaths().

@rsandell rsandell closed this Sep 30, 2015

@rsandell rsandell reopened this Sep 30, 2015

@rsandell

This comment has been minimized.

Copy link
Member

commented Sep 30, 2015

Just tickled the gh pull request builder to try again by closing and reopening.

@rsandell

This comment has been minimized.

Copy link
Member

commented Sep 30, 2015

The build failure seems to be a remoting issue in Jenkins and not due to this change, the maven build itself succeeded.

@scoheb

This comment has been minimized.

Copy link
Contributor Author

commented Sep 30, 2015

Good point. I will rework this one.

Scott Hebert
Permit use of Forbidden files without File Paths
When a job is configured to be triggered with the use of a Forbidden path,
this path is only inspected if the configuration ALSO contains a File path.

This patch corrects this requirement.

[JENKINS-29663]

Change-Id: Ice6db4dc41961d178cc55a97371f95dbb7cec751

@scoheb scoheb force-pushed the scoheb:rework-forbidden-path branch from 6f8ab60 to 53856c6 Sep 30, 2015

@scoheb

This comment has been minimized.

Copy link
Contributor Author

commented Sep 30, 2015

Same remoting issue causing failure?

@rsandell

This comment has been minimized.

Copy link
Member

commented Sep 30, 2015

Yes, same issue it seems.

rsandell added a commit that referenced this pull request Sep 30, 2015

Merge pull request #251 from scoheb/rework-forbidden-path
Permit use of Forbidden files without File Paths

@rsandell rsandell merged commit b5c088b into jenkinsci:master Sep 30, 2015

1 check failed

Jenkins Looks like there's a problem with this pull request
Details
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.