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: versioned api low node compat fix #2970

Merged
merged 3 commits into from
Aug 31, 2021

Conversation

dariakp
Copy link
Contributor

@dariakp dariakp commented Aug 31, 2021

Description

Updating versioned API not to use Object.values and unified runner not to use the node util for deep equality check.

@dariakp dariakp force-pushed the chore-3.7-evergreen-low-node-compat-fix branch from 8c5df03 to b6498e9 Compare August 31, 2021 19:58
@dariakp dariakp marked this pull request as ready for review August 31, 2021 20:10
Copy link
Contributor

@emadum emadum left a comment

Choose a reason for hiding this comment

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

LGTM with one nit 👍

@@ -3,12 +3,13 @@
const expect = require('chai').expect;
const loadSpecTests = require('../spec/index').loadSpecTests;
const runUnifiedTest = require('./unified-spec-runner/runner').runUnifiedTest;
const ServerApiVersion = require('../../lib/core').ServerApiVersion;
const ValidServerApiVersions = require('../../lib/core').ValidServerApiVersions;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const ValidServerApiVersions = require('../../lib/core').ValidServerApiVersions;
const validVersions = require('../../lib/core').ValidServerApiVersions;

Nit, but if you rename this here there's no need for the second declaration further down.

@dariakp dariakp merged commit 1a76618 into 3.7 Aug 31, 2021
@dariakp dariakp deleted the chore-3.7-evergreen-low-node-compat-fix branch August 31, 2021 21:00
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