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

Fix quickstart tests by updating Node version #1214

Merged
merged 9 commits into from
Sep 21, 2021
Merged

Conversation

leina05
Copy link
Contributor

@leina05 leina05 commented Sep 20, 2021

No description provided.

@github-actions
Copy link

github-actions bot commented Sep 20, 2021

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

Copy link
Contributor

@gkaemmer gkaemmer left a comment

Choose a reason for hiding this comment

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

Makes sense

Copy link
Member

@gj gj left a comment

Choose a reason for hiding this comment

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

Why is this necessary?

We use 12.20 in other parts of the CI process b/c certain development dependencies require it (Jest, IIRC), but we test against 10.0.0 here because that's the minimum Node version the library supports.

@leina05
Copy link
Contributor Author

leina05 commented Sep 21, 2021

Why is this necessary?

We use 12.20 in other parts of the CI process b/c certain development dependencies require it (Jest, IIRC), but we test against 10.0.0 here because that's the minimum Node version the library supports.

The minimum version is actually 12.20 according to the README.

It's necessary because the Node quickstart test was failing on 10.0.

@gj
Copy link
Member

gj commented Sep 21, 2021

The minimum version is actually 12.20 according to the README.

That's the minimum version in order to work on the JS language library; the minimum Node.js version for installing the built oso package has been 10.0.0 since introduction.

It's necessary because the Node quickstart test was failing on 10.0.

Why were the quickstart tests failing on 10.0.0?

.github/workflows/test.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
uses: actions/setup-node@v1
with:
node-version: 12.20.0
node-version: 10.24.0
Copy link
Member

Choose a reason for hiding this comment

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

Not consequential here, but a general note: I think YAML removes trailing .0s from numbers, so this will actually get parsed as 10.24 and the latest version that matches 10.24.x will be installed instead of 10.24.0.

@gj gj added the Docs Apply label to PRs that change docs. label Sep 21, 2021
@gj gj merged commit 1eeb549 into main Sep 21, 2021
@gj gj deleted the leina/quickstart_tests branch September 21, 2021 20:27
@github-actions github-actions bot locked and limited conversation to collaborators Sep 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Docs Apply label to PRs that change docs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants