Skip to content
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

ci: fix the Sync workflow #117

Merged
merged 3 commits into from
Jun 1, 2022
Merged

ci: fix the Sync workflow #117

merged 3 commits into from
Jun 1, 2022

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jun 1, 2022

Currently the Sync workflow is failing because GitHub Actions do not have permission to commit to the main branch. Instead, we can ask it to create a PR, which has the upside of making sure the job is not committing ill-formed data to the main branch.

Other changes:

  • Schedule to run the job once a week (I think it's plenty) instead of daily.
  • Use the latest Node.js LTS version rather than v14.x.
  • Use git show instead of a temporary file.

@aduh95 aduh95 requested a review from arcanis June 1, 2022 14:51
.github/workflows/sync.yml Outdated Show resolved Hide resolved
Co-authored-by: Maël Nison <nison.mael@gmail.com>
aduh95 added a commit to aduh95/node that referenced this pull request Jun 1, 2022
Using tags is a security risk, as they can be updated to point to
anything else.

Refs: nodejs/corepack#117 (comment)
aduh95 added a commit to aduh95/node that referenced this pull request Jun 1, 2022
Using tags is a security risk, as they can be updated to point to
anything else.

Refs: nodejs/corepack#117 (comment)
.github/workflows/sync.yml Outdated Show resolved Hide resolved
Co-authored-by: Kristoffer K. <merceyz@users.noreply.github.com>
@aduh95 aduh95 merged commit e810142 into main Jun 1, 2022
@aduh95 aduh95 deleted the fix-sync-gha branch June 1, 2022 22:47
aduh95 added a commit to nodejs/node that referenced this pull request Jun 5, 2022
Using tags is a security risk, as they can be updated to point to
anything else.

Refs: nodejs/corepack#117 (comment)

PR-URL: #43284
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
italojs pushed a commit to italojs/node that referenced this pull request Jun 6, 2022
Using tags is a security risk, as they can be updated to point to
anything else.

Refs: nodejs/corepack#117 (comment)

PR-URL: nodejs#43284
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@aduh95
Copy link
Contributor Author

aduh95 commented Jun 8, 2022

It looks like using a hash breaks the workflow: https://github.com/nodejs/corepack/runs/6741674996?check_suite_focus=true 🤔

danielleadams pushed a commit to nodejs/node that referenced this pull request Jun 11, 2022
Using tags is a security risk, as they can be updated to point to
anything else.

Refs: nodejs/corepack#117 (comment)

PR-URL: #43284
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit to nodejs/node that referenced this pull request Jun 13, 2022
Using tags is a security risk, as they can be updated to point to
anything else.

Refs: nodejs/corepack#117 (comment)

PR-URL: #43284
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit to nodejs/node that referenced this pull request Jun 13, 2022
Using tags is a security risk, as they can be updated to point to
anything else.

Refs: nodejs/corepack#117 (comment)

PR-URL: #43284
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit to nodejs/node that referenced this pull request Jul 12, 2022
Using tags is a security risk, as they can be updated to point to
anything else.

Refs: nodejs/corepack#117 (comment)

PR-URL: #43284
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit to nodejs/node that referenced this pull request Jul 31, 2022
Using tags is a security risk, as they can be updated to point to
anything else.

Refs: nodejs/corepack#117 (comment)

PR-URL: #43284
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
Using tags is a security risk, as they can be updated to point to
anything else.

Refs: nodejs/corepack#117 (comment)

PR-URL: nodejs/node#43284
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants