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!: update workflow #454

Merged
merged 4 commits into from
Aug 23, 2022
Merged

ci!: update workflow #454

merged 4 commits into from
Aug 23, 2022

Conversation

Fdawgs
Copy link
Contributor

@Fdawgs Fdawgs commented Jul 11, 2022

BREAKING CHANGE: Drops support for EOL node 12 and 15; adds support for node 18

This PR:

  • Drops support for EOL node versions
  • Removes Git credentials/SSH keys after checkout as a security precaution by setting persist-credentials: false, they are not used after the initial checkout
  • Enables concurrency; see related docs, this allows a subsequently queued workflow run to interrupt previous runs in PRs
  • Adds a conditional if to the automerge job; this stops the job from running if the user is not Dependabot, saving a few seconds CI run time

closes #427
closes #431
closes #456

@mcollina
Copy link
Owner

@Fdawgs apparently this fails on CI on Node v18. Could you take a look?

@Fdawgs
Copy link
Contributor Author

Fdawgs commented Jul 25, 2022

Not 100% on this one. Seems to be an issue with passing pfx to tls.createSecureContext(). Swapped it out with a generated .pfx file and it throws an error as well:

image

I've looked through the V17 and V18 changelogs and can't spot any breaking changes in this area. 😢

@mcollina
Copy link
Owner

Maybe skips?

@Fdawgs
Copy link
Contributor Author

Fdawgs commented Jul 25, 2022

Maybe skips?

Found the issue: nodejs/node#40672

Have recreated the test pkcs12 file using OpenSSL v3 and it now uses a supported cipher.

@Fdawgs
Copy link
Contributor Author

Fdawgs commented Aug 23, 2022

Don't know if it's worth adding chalk and pretty-bytes to the list of dependencies to not automerge, considering they're now fully ESM?

@mcollina
Copy link
Owner

Don't know if it's worth adding chalk and pretty-bytes to the list of dependencies to not automerge, considering they're now fully ESM?

add them!

@Fdawgs
Copy link
Contributor Author

Fdawgs commented Aug 23, 2022

add them!

Cool beans, all done @mcollina

Edit: CI failure unrelated to this, was fine prior to c69d1e4

Copy link
Owner

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina
Copy link
Owner

CI is failing :(

@Fdawgs
Copy link
Contributor Author

Fdawgs commented Aug 23, 2022

CI is failing :(

There we go!

@Fdawgs Fdawgs requested a review from mcollina August 23, 2022 11:09
@mcollina mcollina merged commit 9cf9b37 into mcollina:master Aug 23, 2022
@mcollina
Copy link
Owner

thx

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.

2 participants