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

all: bump to use and support node20 #16

Merged
merged 1 commit into from
Feb 20, 2024
Merged

all: bump to use and support node20 #16

merged 1 commit into from
Feb 20, 2024

Conversation

itkq
Copy link
Contributor

@itkq itkq commented Jan 29, 2024

@myitcv
Copy link
Member

myitcv commented Feb 20, 2024

@itkq - thanks for raising this PR, and apologies for the delay in getting to this.

Looks like we have a build failure if you can take a look at that?

@myitcv
Copy link
Member

myitcv commented Feb 20, 2024

I have just pushed a commit to this PR that consolidates all the changes you made, @itkq, into a single commit. I suggest we use that as the basis for merging. I'm looking at fixing the build failure now.

* Upgrade 'vendored' GitHub action schema to use new node20 value.
* Use the latest version of actions/setup-node and actions/checkout in
  the workflows for this project to avoid triggering warnings about
  using node16-based actions.
* Upgrade the action that is this repo to be node20-based.
* Upgrade the version of CUE used in this repo to v0.7.1, current
  latest.
* Bump and fix various node dependencies to get things working again.
* Run 'npm ci' in workflows for this project to be more robust.

This change also disables the running of the jest tests as part of CI.
They are currently broken and myitcv does not have the experience to
debug and fix them. We still have coverage of the code however, because
the CI workflow for the project itself use the action to install CUE as
part of every commit.

Signed-off-by: Takuya Kosugiyama <[email protected]>
Co-authored-by: Paul Jolly <[email protected]>
@myitcv myitcv changed the title Bump to use node20 runtime all: bump to use and support node20 Feb 20, 2024
@myitcv myitcv requested a review from jpluscplusm February 20, 2024 15:52
@myitcv
Copy link
Member

myitcv commented Feb 20, 2024

The build failures are fixed, but the tests are failing for some reason. I have worked around this by disabling the tests as part of CI. Per the commit message, this is relatively low risk, on the basis the action itself is tested as part of the CI workflows in this repo. So for every commit, we are testing that the action behaves.

Copy link
Collaborator

@jpluscplusm jpluscplusm left a comment

Choose a reason for hiding this comment

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

I don't /see/ anything wrong with this, but I've by no means performed an exhaustive review (and am not well placed to do so, without spending non-trivial time).

@itkq
Copy link
Contributor Author

itkq commented Feb 20, 2024

@myitcv Thank you for reviewing this PR and for making the necessary commits!

@myitcv myitcv merged commit a93fa35 into cue-lang:main Feb 20, 2024
2 checks passed
@itkq itkq deleted the node20 branch February 21, 2024 00:59
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