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

feat: relay jenkins and gh events to gh #272

Closed
wants to merge 4 commits into from

Conversation

mmarchini
Copy link
Contributor

@mmarchini mmarchini commented Aug 2, 2020

Initial implementation of the "Actions relay" suggested here: #264. Doesn't work yet because createDispatchEvent is not available on the github version we're using.

(current PR rebased on top of #271 and #270, actual implementation here)

`github` has been renamed to `@octokit/rest`. The version sequence was
kept, and the package name is the only breaking change on v14.0.0.

Ref: https://github.com/octokit/rest.js/releases/tag/v14.0.0
Only breaking change on v15.0.1 is to `DELETE` calls, which we don't
use.

Ref: https://github.com/octokit/rest.js/releases/tag/v15.0.1
@mmarchini
Copy link
Contributor Author

Ah, one thing we need for GitHub events is to differentiate events coming from people with write access to the repo vs people without write access to it. We can append a ! when the event has write access or smth (dispatch event will always have write access in the action, which is fine, but it's good for us to filter some actions so they only run for people with the right permissions).

@phillipj
Copy link
Member

phillipj commented Aug 9, 2020

That sounds convenient indeed! 👍

@mmarchini
Copy link
Contributor Author

Oh nice, the GitHub part of this PR was rendered unnecessary by GitHub earlier this week: https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/. With the new event announced in this blog post we can move everything we have here to Actions on nodejs/node, except for the Jenkins status updater, which will still need the relay.

@phillipj
Copy link
Member

Hooray! Cool to see how that in practise made CI-start-on-label in nodejs/node (nodejs/node#34707) a lot simpler as well 💯

@mmarchini
Copy link
Contributor Author

I take it back, the new event is still not enough for some use cases (basically any use case that is not a check/linter/test), so we still want the relay

@mmarchini
Copy link
Contributor Author

FYI I'll break this into two PRs: one for the Jenkins relay and one for the GitHub relay. Starting with the Jenkins relay which I think is more straightforward. This will allow us to experiment and tweak with it before adding GitHub as well. Also, if it works as expected we'll be able to remove a good chunk of code once we move Jenkins PR status to Actions :D

@phillipj
Copy link
Member

Good idea! I'm a big fan of ship-small-and-tweak 👍

@phillipj
Copy link
Member

phillipj commented Nov 2, 2020

@mmarchini you okey with me picking up the jenkins relay work you mentioned if I find the time?

Would be cool to contribute to the recent GitHub Actions efforts and move us in a direction we touched upon in the modernisation issue (#264).

@mmarchini
Copy link
Contributor Author

I'm just back from vacation and was planning on picking this up again in a few weeks :)

If you have time before that, feel free to pick it up.

@phillipj
Copy link
Member

phillipj commented Nov 2, 2020 via email

@phillipj
Copy link
Member

Just opened #289 with one of the commits from this PR cherry picked & slimmed to only relay Jenkins events for now.

@Trott
Copy link
Member

Trott commented Aug 25, 2022

I'm going to close this as stagnant but by all means re-open if someone is going to be working on it. Thanks.

@Trott Trott closed this Aug 25, 2022
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.

3 participants