Skip only if all changes does skip with passing unit tests. #7
Conversation
Imagine the following commits messages - [skip ci] skip this - do not skip this The build will be skipped, and the next commit will be like this : - [skip ci] skip this - do not skip this - really do not skip Se the build will be skipped and so on. Fixes https://issues.jenkins-ci.org/browse/JENKINS-63918
| for (Object entry : changeLogSet.getItems()) { | ||
| if (entry instanceof ChangeLogSet.Entry && inspectChangeSetEntry((Entry) entry, matcher)) { | ||
| matchedEntry = (Entry) entry; | ||
| } else { | ||
| allSkipped = false; | ||
| break; |
There was a problem hiding this comment.
@worldjoe With this change, aren't you checking the first commit instead of the last?
There was a problem hiding this comment.
@debugmaster I looked at it too, but I think it is correct.
The "if else" is not testing whether there is some (or more) ci skip. It is testing whether there is some commit WITHOUT ci skip. If there is any, the allSkipped is set to false and the loop does not need to continue.
I think we don't want to skip build if there is some commit without ci skip.
BTW: matchedEntry is not used anymore, so matchedEntry = (Entry) entry; can be removed (and if condition inverted if it will be more readable).
There was a problem hiding this comment.
I see. If all commits match the pattern, then skip run. Otherwise, don't skip.
|
This can solve my issue, but I noticed that this is a PR submited 2 years ago, anyone can help merge this and release it? |
|
Hi @plavc 👋 😄 Are you still working on this plugin? Any chance you would have time to take a look at this? |
|
I did some refactoring that should resolve the issue above, but I didn't have time to finish. |
|
Hi @plavc 😄 |
|
@plavc @debugmaster are you looking for new maintainers? |
removed unused matchedEntry. reduce if/else to just if not.
|
Thanks! |
This is a cleanup of this pull request: #6
I didn't know how to push to that pull request. But feel free to close this and just incorporate there.
That pull request had 2 problems:
Thanks to @jdeniau for the original code. I just made minor changes.
Fixes https://issues.jenkins-ci.org/browse/JENKINS-63918