-
Notifications
You must be signed in to change notification settings - Fork 36
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
Migrate to GitHub actions #77
Comments
It looks like we can run Node.js code there, so as much as I'd like to rewrite this tool, we don't necessarily have to. That said, there are some reasons to skip this. For one, it increases WPT's lock-in to GitHub.com (the current solution relies on "webhooks" which are offered by other hosting providers). Also, GitHub takes the "beta" status seriously, quietly removing features, drastically changing file formats, and apparently deploying broken functionality. It may be wise to wait until the feature is out of beta before committing more of our infrastructure to it. |
I agree we might want to be a bit careful about depending on GitHub Actions for things that are critical to the functioning of the WPT project, taking the advice from https://help.github.com/en/articles/about-github-actions:
However, I think our GitHub lock-in is very strong already, mostly because of use of the GitHub API that would require rewriting if we ever migrated to something else. The webhook payload that wpt-pr-bot depends also seems GitHub-specific, GitLab's data structures are subtly different. In other words, if there are other non-trivial benefits, I'd be happy to make it even harder to migrate away from GitHub. |
Spotted in https://github.blog/2019-08-08-github-actions-now-supports-ci-cd/:
So, we need not wait for long before we know if it'll fly or not. |
Also, it's probably a good idea to try our workflows while it is in beta, because bugs we report during beta are more likely to be fixed than after it's "done". |
I think there will be challenges with permissions here, and these issues seem to confirm it: To be clear, we wouldn't use https://github.com/actions/labeler anyway, but we have the same constraints. |
@foolip We might want to abstract the approach that we developed for wpt.live. WPT could define a "scheduled" GitHub Action that only simulated pull request events using the generic Unlike the authentic pull request events, all events simulated like this would have write access (not just those from trusted contributors). This would be safe because the Actions would be running from the Both the wpt.live submissions Action and the wpt-pr-bot Action could be expressed as consumers of these synthetic events. |
That would probably work, but I fear that any polling approach would be too slow when it comes to adding labels and reviewers. But maybe wpt-pr-bot could be slimmed down into something that only pokes at GitHub actions in response to webhooks... |
It makes me wonder if GitHub has considered offering this kind of functionality directly. Maybe if the Workflows defined in the project's default branch were given some special status, then they could safely be granted write access for pull requests from untrusted contributors. This wouldn't be helpful for validating the patches themselves, but it seems like there's a general need for automating administrative tasks. I don't know an effective channel for feature requests, though. |
The general feedback form for GitHub Actions beta is probably a good place. I would guess that someone is already painfully aware of this, since GitHub Actions was originally tailored to workflow automation cases like this, and then pivoted to more traditional CI/CD seemingly when they switched the backend to Azure. That broke some of the first-party actions like the labeled... A fix will probably come eventually, so a workaround in the meantime seems acceptable. |
GitHub Actions is in public beta now, and looks like it's going to be a very good fit for the things that this bot is responsible for. For example https://github.com/actions/labeler adds labels to pull requests, although we probably wouldn't be able to use that because our rules are too complex.
Benefits:
The text was updated successfully, but these errors were encountered: