-
Notifications
You must be signed in to change notification settings - Fork 3
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
Enhance ruleset event support #159
Comments
I have a use-case for this as well. For pull request alone there are a number of (sub) events we could support. refs: |
Adding an additional request for PR approval event. refs: |
|
Given some of the feedback on the initial types PR, I think it would be a good idea to discuss overall implementation of this feature in greater depth. YAML IdeasWith the inclusion of more events, many of them sub-events, we have to consider how this will affect the pipeline writing for our end users. I have a few markups for how this might work, which hopefully will generate some discussion as to which direction we want to take. Mark Up 1 (putting sub-events on same level as normal events)steps:
- name: original behavior
ruleset:
event: [push, pull_request]
- name: original behavior using sub-events
ruleset:
event: [push, pr_open, pr_commit]
- name: example customized event set
ruleset:
event: [push, pr_edit, pr_label]
label: bug Mark Up 2 (scoping sub-events of PRs)steps:
- name: original behavior using scoping
ruleset:
event: [push, pull_request:open, pull_request:commit]
- name: example customized event set
ruleset:
event: [push, pull_request:edit, pull_request:label]
label: bug Mark Up 3 (unrolling events array into a YAML structure like
|
My personal preference is towards option 2 as it seems to look and act more like OAuth Scopes within GitHub. It (at a glance) seems like it might make the migration path easiest for users as their existing pull_request event will be the highest level category but for finer grained control users can only look for specific sub-events. |
I think I personally lean towards option two as well. Largely because I think it would be the easiest for us to implement with causing to much headache on the user front. I do really like the verbosity of three but I'm not just not sure the way all that YAML parsing would work it would be super easy to implement. Yeah, the oauth scopes in GitHub do draw a nice similarity. Sadly, I don't think all users can benefit seeing as a Vela installation might not run against GitHub. Is the plan with this to tackle more than just Pull Request additions? I see label, comment and some others listed in the issue. |
I think option 2 is the one I'd like to see us use for Vela in the future.. but I'm not sure it's the right approach currently. In order to maintain the precedent and standards already set forth for a Again, I think option 2 is likely the best candidate and we should strongly consider it for a But, option 3 fits with the current patterns we've established and I'll provide some examples: ruleset:
event: [ comment ]
comment: [ <myComment> ]
ruleset:
event: [ deployment ]
target: [ <myDeployTarget> ]
ruleset:
event: [ tag ]
tag: [ <myTag> ] The pattern here is the And if further customization beyond the i.e. what comment I want Vela to run the step for, what deployment target I want Vela to run the step for etc. I think most, if not all, would agree the pattern we have today does leave room for opportunity. However, it is a pattern already established so we should be mindful about deviating from it. How I see this working with the current pattern we have today would look like: ruleset:
event: [ pull_request ]
action: [ <myPRAction> ]
ruleset:
event: [ pull_request ]
label: [ <myPRLabel> ] Depending on the level of support we want for ruleset:
event: [ label ]
label: [ <myPRLabel> ] I say this because IIRC, we had to introduce the I believe that's why we have a separate story written for adding support for labels: |
With the release of While not all events mentioned here are covered with this release, I am closing this issue in favor of multiple specific requests for webhook events: PR Label and PR Review Submission to get us started. |
Description
event
inruleset
.Scenario:
By default when opening a PR it'll target our default branch (main) and if somebody is aiming to cherry-pick their PR into the release branch they'll edit the PR to retarget the new base branch. In this case, there is no build that gets triggered because Vela only supports few events including
pull_request
,commnent
,tags
,push
, etc.What I would be imagining is that anything that is "action": "edited" triggers a rebuild.
Value
This is important because in the worst scenarios, the stale-ness of the checks means that bad commits make it into the repo based on some merge commit from a branch other than what it would move into.
Definition of Done
Effort (Optional)
Impacted Personas (Optional)
The text was updated successfully, but these errors were encountered: