-
Notifications
You must be signed in to change notification settings - Fork 82
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
Spec PR review action - POC #421
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Lukas Jungmann <[email protected]>
✔️ Deploy Preview for jakartaee-specifications ready! 🔨 Explore the source changes: e2bc26c 🔍 Inspect the deploy log: https://app.netlify.com/sites/jakartaee-specifications/deploys/613a0c0e349337000890d8ad 😎 Browse the preview: https://deploy-preview-421--jakartaee-specifications.netlify.app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest we avoid checking in the large number of node_modules files by following the suggestion in the following doc to produce a single index.js file using @vercel/ncc:
https://docs.github.com/en/actions/creating-actions/creating-a-javascript-action
Whatever you think is the best, I only need to know where to (re-)create this PR... One drawback I can see is in syncing changes between those two repos. On the benefits side, "test" won't spam this repo and it may be easier to distinguish between dev and stable versions. Compiled version should also run faster - but I do not think speed matters that much in this particular case. ...and forgot to mention that for current testing, one can use my repo: https://github.com/lukasj/repo |
So after I had a base rewritten in typescript I started trying to test it in my fork repo, I ran into problems due to the issues described here: Since creating PRs from forks is how we do reviews, it seems github actions are unusable, however I'm not clear on if the problem is because I'm testing in the fork repo. Maybe we need to merge a simple workflow to test if an action is able to add/update comments on the specifications repo. Now I'm just looking at creating an action using the following rest api java binding: This can be run externally, or via docker I suppose. |
Here is an example of doing the same work to check a PR and update a comment as the action was doing using the org.kohsuke:github-api GitHub REST binding for Java. Of course this needs to be run manually rather than from a workflow event. The work to update @lukasj POC into TypeScript is available in the actions branch of this repo: It only has the first check regarding conformance to the PR template ported since I could not get past the permission errors to create a comment from this forked repo. |
Defines a workflow for verification of specification PRs.
The workflow is currently set to react on PRs to master/main branch IF the PR description contains Specification PR template. It also runs after every modification of such PR.
The "custom" nodejs based action included here takes a list of modified files within the PR and PR description as an input, goes through the 1st group of checks defined by the spec_review_checklist.md and outputs findings as MD formatted text
The workflow then adds this output as a comment to the PR (the comment is either created or updated after each modification of the PR/action run)