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

Draft
wants to merge 4 commits into
base: master
from

Conversation

@mmarchini
Copy link
Member

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)

mmarchini added 4 commits Aug 1, 2020
`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
Member Author

mmarchini commented Aug 9, 2020

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
Member Author

mmarchini commented Aug 10, 2020

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

phillipj commented Aug 10, 2020

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
Member Author

mmarchini commented Aug 11, 2020

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
Member Author

mmarchini commented Aug 15, 2020

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

phillipj commented Aug 17, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.