-
Notifications
You must be signed in to change notification settings - Fork 0
feat: use actions/github-script instead of running node directly #27
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,22 +7,23 @@ import { setTimeout } from 'timers/promises'; | |
| const tries = 10; | ||
| const retryDelay = 30000; | ||
|
|
||
| await setTimeout(retryDelay); | ||
| export default async function(github, owner, repository, pull_number) { | ||
| const repo = repository.replace(new RegExp(`${owner}/`), '') | ||
| await setTimeout(retryDelay); | ||
|
|
||
| for (let t = 0; t < tries; t++) { | ||
| try { | ||
| const [repo, pull_number] = process.argv.slice(2); | ||
| for (let t = 0; t < tries; t++) { | ||
| try { | ||
| const { data } = await github.rest.pulls.get({owner, repo, pull_number}) | ||
|
|
||
| const data = await (await fetch(`https://api.github.com/repos/${repo}/pulls/${pull_number}`)).json(); | ||
|
|
||
| console.log(data); | ||
| if (data.mergeable_state === 'clean') { | ||
| process.exit(0); | ||
| console.log(data); | ||
| if (data.mergeable_state === 'clean') { | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wasn't really able to test this one on my branch as I had issues getting the test action to run for some reason... How did you test?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created a new repo with some simple js scripts and a small test suite, kept polling the GitHub API until the mergeable_state became 'clean'. At creation, it's set to 'unknown'. Then it goes 'unstable' until checks pass, which set that variable to 'clean'
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Repo was public, of course
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, but it's never clean due to missing approval, and if approval is added, it might be mergable right away. I think. Anyways, if automerge doesn't work, we can iterate. Getting the PR to begin with is a great start as it can be merged from mobile, no need to run the script from desktop |
||
| process.exit(0); | ||
| } | ||
| await setTimeout(retryDelay); | ||
| } catch (error) { | ||
| console.error(error); | ||
| process.exit(1); | ||
| } | ||
| await setTimeout(retryDelay); | ||
| } catch (error) { | ||
| console.error(error); | ||
| process.exit(1); | ||
| } | ||
| process.exit(1); | ||
| } | ||
| process.exit(1); | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
might be enough to do
const [owner, repo] = repository.split('/')? not sure if it's legal with other/which can mess this upThere 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.
Can't think of a one-liner for that. I would split, use the first part and join the other parts.
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.
Seems illegal, so just splitting should be fine:
