-
Notifications
You must be signed in to change notification settings - Fork 8
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
Write the first action version #1
Conversation
src/entrypoint.tsx
Outdated
client | ||
) | ||
if (reviewCount < requiredReviews) { | ||
toolkit.exit.failure( |
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 guess this raises an exception that is not catched and break execution flow, otherwise we need an else
src/entrypoint.tsx
Outdated
} | ||
|
||
Toolkit.run(async (toolkit: Toolkit) => { | ||
toolkit.log.info('Running Action') |
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 refactor out this run method to the lib so we can test it mocking the toolkit
d59cc63
to
248ca88
Compare
798ae70
to
c5b455a
Compare
This is the first version of our first custom Github action.
The purpose of this action is to scan for certain labels on the PR, then check for a configuration file that will be present on the repo with pairs like
migration, 4
,typescript, 5
etc... and then if there is a match it will check if the PR has been approved by at least the highest matching number of reviewers.Why is this based on labels? So this can be plugged-in with other open-source github actions we already have like the one that auto-labels the PR's based on the files that are being commited. By pluggin-in these two we can create things like "if a PR contains a migration file, tag the file as
migration
and do no let it be merged until approved by 2 people"Ideally we could opensource this action too so other people can use it in their workflows. I'm syncing with @javiertejero tomorrow morning about this.
But first I'm creating this mainly for @giuband , there is some stuff I'll like to be reviewed, specially the javascript/typescript flavor, I just adopted some configurations from somewhere else and these are not our standards, so I would be completely ok if we decide to setup different customization.