Skip to content

feat: Add auto-pr cron job #1314

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

feat: Add auto-pr cron job #1314

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
526c9ef
feat: Add auto-pr cron job
nschonni Aug 15, 2020
d0de477
fix: Add GITHUB_TOKEN passing
nschonni Aug 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/workflows/auto-pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
name: Auto PR for new versions

on:
schedule:
- cron: 0 0 * * *
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a comment about what this means so I don't have to use https://crontab.guru/?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- cron: 0 0 * * *
- cron: 0 * * * *

Every hour instead of every day? Otherwise I assume people will be posting issues wondering when the next release will be available if it's potentially 23 hours delayed.

Could run it event more often of course, not like it takes a lot of resources/time

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nschonni I still think this makes sense, fwiw

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think every hour is better 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or even more often - any issue running this every 10 minutes?


jobs:
update:
strategy:
matrix:
version:
- 10
- 12
- 14
Comment on lines +11 to +14
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will create 3 PRs if there are updates for all release lines, right? Should we instead just do ./update.sh without passing a version and use some node script to construct the PR title and body?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

./update.sh is broken atm and just runs against all versions even when you pass one as an argument.
Also, this will create PRs for yarn updates too even when there isn't a node update, since it doesn't have the -s flag.

I am interested in fixing update.sh or even perhaps porting it to a node script, if that is desired. It could be made more useful like returning what node versions it updated, etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

migrating to a js script makes sense to me.

As a temporary workaround, the script could run with -s first, and only if there is a diff, run again without the -s. But yeah, might make more sense to port it to a js script so it's a bit easier to work with more advanced logic?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should happen if there is a keys update (like there was a few days ago). Should any images be updated if there are no node updates?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, it should only create a PR if the Node version line changes. Key changes should only go in with a version bump

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I'll add some logic to grab the keys in the existing files to replay them on the templates, unless the node version changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can probably just try looking at the git diff result

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should only create a PR if the Node version line changes

There might be bug fixes and such, so I think it's better to say that

  1. yarn version change
  2. key changes

only triggers an update if the node version changes - all other changes than those two should always trigger

runs-on: ubuntu-latest
steps:
- name: Update Docker images
uses: technote-space/create-pr-action@v2
with:
EXECUTE_COMMANDS: |
./update.sh ${{ matrix.version }}
COMMIT_MESSAGE: 'chore: update ${{ matrix.version }}'
COMMIT_NAME: 'Node.js GitHub Bot'
COMMIT_EMAIL: 'github-bot@iojs.org'
PR_BRANCH_NAME: 'chore-update-${{ matrix.version }}'
PR_TITLE: 'chore: update ${{ matrix.version }}'
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}