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

Prevent triggering builds on closed PRs [DI-248] #40

Conversation

JackPGreen
Copy link
Contributor

@JackPGreen JackPGreen commented Aug 19, 2024

Skips all kind of event triggers for closed PRs, rather than just for comments as per jenkinsci/ghprb-plugin#54.

I've not added a test to cover this, as there's no existing test coverage for pull_request events in org.jenkinsci.plugins.ghprb.GhprbRootActionTest, nor is there any example payloads in org.jenkinsci.plugins.ghprb.GhprbTestUtil.

Fixes: jenkinsci/ghprb-plugin#865, DI-248

Post-merge actions:

  • Deploy in Jenkins
  • Test
  • Raise upstream PR

Skips all kind of event triggers for closed PRs, rather than just for comments as per jenkinsci/ghprb-plugin#54.

Fixes: jenkinsci/ghprb-plugin#865
@JackPGreen JackPGreen marked this pull request as draft August 19, 2024 16:25
@JackPGreen JackPGreen changed the title Prevent triggering builds on closed PRs Prevent triggering builds on closed PRs [DI-248] Aug 27, 2024
@JackPGreen JackPGreen marked this pull request as ready for review September 3, 2024 14:49
@nishaatr
Copy link

nishaatr commented Sep 3, 2024

General questions

Deploy in Jenkins

How? just want to know

Test

So the change has not been tested?

Raise upstream PR

do we push changes to upstream always?

@JackPGreen
Copy link
Contributor Author

General questions

Deploy in Jenkins

How? just want to know

Process is outlined in the README

Test

So the change has not been tested?

No, there's no way I can test it locally.
(well, not easily at least).

Raise upstream PR

do we push changes to upstream always?

Don't know, but seems the right thing to do.

@nishaatr
Copy link

nishaatr commented Sep 3, 2024

Don't know, but seems the right thing to do.

yes would be but this doesn't look like a fork

@JackPGreen
Copy link
Contributor Author

Don't know, but seems the right thing to do.

yes would be but this doesn't look like a fork

It's not a fork, but we can still do it.

@nishaatr
Copy link

nishaatr commented Sep 3, 2024

Don't know, but seems the right thing to do.

yes would be but this doesn't look like a fork

It's not a fork, but we can still do it.

yes true. but looks like we never pushed upstream changes and reading Slack comments, might not be worth the effort.
Also last master commit in orig repo was ~2yrs ago and there are a few open PRs.
No strong opinion but if we don't do upstream push then best to update PR description

Copy link
Contributor

@kwart kwart left a comment

Choose a reason for hiding this comment

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

LGTM

@JackPGreen
Copy link
Contributor Author

@kwart thanks, are you able to merge, please?

Only those with write access to this repository can merge pull requests.

@kwart kwart merged commit 0d8b1fe into hazelcast:hazelcast-patches Sep 4, 2024
4 checks passed
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.

Prevent triggering builds on closed PRs
3 participants