Feature: Option to report success to PR when build is not triggered

[MaitreyaBuddha]

This is a resolution for #630

This introduces a new checkbox after include/exclude sections. It gives you the ability to report GHCommitState.SUCCESS status with "Skipped..." message when a build is skipped because it didn't match any include/exclude patterns.

For example, if you have 4 builds but only Java files changed, you might get this output:

The title of this PR should work for CHANGELOG entry.

[bjoernhaeuser]

Thank you very much for your contribution!

[bjoernhaeuser]

hm, shouldnt that method already exist somewhere else?

[bjoernhaeuser]

Ah I see the problem. So basically you cannot call this method.

Should this method be extracted and moved to a common place or is there no abstraction level which makes sense for both callers?

I personally feel its a bit edgy that the interface now has the createCommitStatus method.

[MaitreyaBuddha]

You're right, it is out of place there the way it is. I wanted to make an easy implementation of GhprbPullRequest.createCommitStatus(GHCommitState state, String message) for future developers, but maybe that's not necessary.
I could create GhprbCommitStatus.onBuildSkipped that would make more sense for GhprbCommitStatus being an event interface. But also, the only other implementor is GhprbNoCommitStatus and completely unused. I could remove the interface, remove GhprbNoCommitStatus, and then it doesn't seem as edgy.

I thought that level of refactor might be out of scope, but I'm open to it. Of these options I am in favor of removing the unused classes & abstraction. Which direction would you prefer?

[bjoernhaeuser]

Sorry for the late response. I think the best option is to remove the unused classes & abstraction.

Do you have an Idea in your head how this could look like in the end?

[MaitreyaBuddha]

Just pushed it. It was relatively concise and simple. It exposed to me that there's another class GhprbSimpleStatusDescriptor which is completely unused, but I left that as it is not in scope of this change.

[bjoernhaeuser]

@samrocketman I this is fine for you I would go ahead and merge this and test the current master with the other PRs which are "candidate to merge" and are approved by either me or you. Is this fine for you?

[samrocketman]

@bjoernhaeuser you're welcome to go ahead; I haven't had a chance to look at it but if you feel it's good to merge I don't have any problem with it.

[MaitreyaBuddha]

Sorry for the noise, we prematurely merged unrelated work to our fork's master. It has been reverted and this is back to the latest relevant changes.

[bjoernhaeuser]

@MaitreyaBuddha could you squash your commits into one? :)

[MaitreyaBuddha]

@bjoernhaeuser Done! Github also has an option to do that when merging if you have not disabled it in settings.

[bjoernhaeuser]

@MaitreyaBuddha would be cool if you can rebase to latest master to retrigger the jenkins build itself. For whatever reason it failed :(

[MaitreyaBuddha]

@bjoernhaeuser Rebased and passed 💥

[samrocketman]

We're planning to release this upcoming Monday and won't be merging any additional changes until after Monday's release (Apr 4th).

[miguelslemos]

Any update on this issue?

[samrocketman]

This change can’t be merged or tested without passing checks.

[MaitreyaBuddha]

Rebased to master to trigger build again. Looks g2g?

[samrocketman]

I'll look it over again.

[bjoernhaeuser]

@samrocketman IMHO we can merge this :)

[MaitreyaBuddha]

I had to create another fork and PR to trigger build again: #739

[bjoernhaeuser]

@samrocketman also with this, can we merge it?

[mmpetarpeshev]

Can we merge this and release new version ?

[asif-anwar]

Any update on this one.