On Mon 19 Mar 2018 at 08:39, Stephen Connolly < [email protected]> wrote:
> On Mon 19 Mar 2018 at 08:13, Stephen Connolly < > [email protected]> wrote: > >> >> On Mon 19 Mar 2018 at 00:50, Steven F <[email protected]> wrote: >> >>> I locally modified GHBS to make the constructors for PullRequestSCMHead >>> and PullRequestSCMRevision public which has seemingly solved the problem. >>> >> >> What did I do in Gitea plugin? >> >> If I made the constructor public there, then that’s what we should >> probably be doing in GitHub/ Bitbucket >> >> (Gitea is my “no hacks or legacy migration complexity reference >> implementation”) >> >> So to be clear: >> >> 1. You have a filter trait that restricts PRs based on the >> presence/absence of a number of labels. With that, scanning should work. PR >> commit updates should work (ie if the PR meets the label criteria and >> someone updates the commit) >> 2. You want to have an event listener for PR labelled and unlabelled >> events. This event listener will return heads with a revision when the PR >> starts matching the label criteria and heads with a null revision when they >> stop. All other PR events can continue to be processed as before >> >> Ideally you’d allow things like: >> >> * only discover PRs labelled with “needs-check” >> * ignore PRs labelled with “work-in-progress” >> >> And a final question, would a BranchBuildStrategy work better (ie >> discover all PRs but only build one with the matching label criteria) - >> more asking to see what your use case is >> > > Thinking out loud: > > The branch build strategy would allow all PRs to be discovered, but only > build the ones with the label criteria match. As such, it could miss some > of the “labelled” and “unlabelled” changes unless the branch revision has > also changed since last build... need to think that through > Ok thought experiment: PR has label criteria already, revision updated, so because revision != last build revision => consult strategy PR has label criteria and transitions to no label criteria, so no revision change => no need to rebuild anyway PR does not have label criteria, revision updated and still no label criteria. Because revision != last build revision => consult strategy (which says “no build” because does not meet label criteria) PR does not have label criteria and transitions to having label criteria. This is the case where we could have issues... now would we? If no previous build (should be the normal case) revision != last build revision => consult strategy If previous build (because there was a manual build, or the label was removed & added again): we may have the case where revision == last build revision, in which case, no build triggered. So we can lose build events if either the label criteria is broken and restored without a revision change to the PR or if a manual build of the PR was triggered *before* the label criteria is established. Neither case seems particularly worrisome as they are “defensible”... Though, if the user has configured the Jenkinsfile to behave differently depending on the labels, the user *may* want to trigger a rebuild on label change... but perhaps we can address that in branch-api by consulting the branch build strategies even when the revision matches the last built revision... NOTE TO SELF: TL;DR probably do not need to add labels to PullRequestSCMHead, rather return them as an action from fetchActions... that would be much less risky for build storms > Another approach could be to add the labels to the PullRequestSCMHead as a > mixin. This would make the label info part of the equality contract, so > changing labels would trigger a rebuild... but would enable categories > based on PR labels... and a BranchBuildStrategy could suppress the rebuild > on label change. > > In any case, I see a cohort of users who do not want to have PRs that do > not match the label criteria to even be displayed (even though displaying > without auto-build would allow manual builds of PRs without label criteria, > the users may not want the distraction) so I see a SCMFilterTrait and > corresponding event listener to fill the event gap as a Good Thing™️, but > we may need think about the other lines of attack for the other cohorts of > users too! > > >> (I think it would be really cool if you get this enabled as an extension >> plugin By The Way, and kudos to you) >> >> So: >> >> * if gitea has public constructors for PR head and revision, please file >> a PR against GitHub to make constructors public. Mention that this PR is >> “an enabling change for extension plugin” >> * check that your extension plugin is only filling the event gap (ie >> labelled and unlabelled events) otherwise it will conflict with the other >> events >> >> Really cool feature. Glad the docs helped you. Please file PRs for any >> docs improvements you can think of. >> >> >> It is also correctly scoping the source request to the PR involved in the >>> event. I don't know enough about GHBS to know if exposing these >>> constructors would cause issues though. It seems like the package-private >>> decision was made here >>> <https://github.com/jenkinsci/github-branch-source-plugin/commit/996cd93f9146329c34791b6db4a3e42e6e32c4a8#diff-27ddccdb50cf2a720dcbf7561d45c9edR45> >>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "Jenkins Developers" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to [email protected]. >>> To view this discussion on the web visit >>> https://groups.google.com/d/msgid/jenkinsci-dev/c8805fee-82e4-4fbd-861b-586cfae41c58%40googlegroups.com >>> <https://groups.google.com/d/msgid/jenkinsci-dev/c8805fee-82e4-4fbd-861b-586cfae41c58%40googlegroups.com?utm_medium=email&utm_source=footer> >>> . >>> For more options, visit https://groups.google.com/d/optout. >>> >> -- >> Sent from my phone >> > -- > Sent from my phone > -- Sent from my phone -- You received this message because you are subscribed to the Google Groups "Jenkins Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CA%2BnPnMz4pYPQSQunMC_hxsuMdzSZ5cVvpJMeBjpvyN3P7vDbLA%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
