Skip to content

Conversation

@nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Feb 1, 2022

Description

What is changing?

  • Dependency updates:
 bl                       // removed
 node-addon-api           ^4.1.0  →   ^4.3.0
 prebuild-install          6.1.2  →    7.0.1  // major, needed, drops support for node 10
 bson                     ^4.4.0  →   ^4.6.1
 chai                     ^4.3.4  →   ^4.3.6
 clang-format             ^1.5.0  →   ^1.6.0
 eslint                  ^4.19.1  →   ^8.8.0  // major, we use a version of node that can run this
 eslint-plugin-prettier   ^2.7.0  →   ^4.0.0  // major, tooling sync 
 jsdoc-to-markdown        ^5.0.3  →   ^7.1.1  // major, ran the docs gen and it still works
 mocha                    ^4.1.0  →   ^9.2.0  // major, tooling sync
 mongodb                  ^3.6.9  →   ^4.3.1  // We'll now use this version in CI by default
 node-gyp                 ^5.1.1  →   ^8.4.1  // major, drops python2 support, node 12+
 prebuild                ^10.0.1  →  ^11.0.2  // major
 prettier                ^1.19.1  →   ^2.5.1  // major, does cause reformatting, tooling sync
 sinon                    ^4.5.0  →  ^13.0.1  // major, tooling sync
 sinon-chai               ^3.6.0  →   ^3.7.0
 standard-version         ^9.3.0  →   ^9.3.2
 tar                      ^6.1.0  →  ^6.1.11

This tooling sync and updates change brings:

  • mocha, eslint, prettier configs
  • update nvm and windows-nvm
  • bring in our test reporter for EVG, w/ skipReason functionality
  • run lint in CI
  • move index.js into the lib folder (aligned with driver, less special arguments for linter)
  • Use a copy of our BufferPool implementation from the driver, I brought the tests with it

There is no Bug fix:

  • stateMachine.js typo-ed localhost (my mistake, it was correct)

What is the motivation for this change?

  • We want to have a consistent development environment across our packages.
  • We want to bring in major version bumps of our dependencies before making the major release

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: <type>(NODE-xxxx)<!>: <description>
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket - need to make ticket stateMachine.js:239

Comment on lines -317 to -318
request.addResponse(buffer.slice(0, bytesNeeded));
buffer.consume(bytesNeeded);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our read method will accomplish the same thing in one call, returning a new copy of the bytes and dropping them from the underlying bufferpool.

@nbbeeken nbbeeken force-pushed the NODE-3849/deps branch 2 times, most recently from 6a8bcf7 to 1b11893 Compare February 1, 2022 23:17
Co-authored-by: Anna Henningsen <anna@addaleax.net>
@nbbeeken nbbeeken added dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code labels Feb 2, 2022
@nbbeeken nbbeeken marked this pull request as ready for review February 2, 2022 16:21
Copy link
Contributor

@dariakp dariakp left a comment

Choose a reason for hiding this comment

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

Just some questions here

@nbbeeken nbbeeken requested a review from dariakp February 4, 2022 15:37
Copy link
Contributor

@dariakp dariakp left a comment

Choose a reason for hiding this comment

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

LGTM, but maybe we should get at least one more person from the team to review before we merge

@durran durran merged commit b40d845 into master Feb 7, 2022
@durran durran deleted the NODE-3849/deps branch February 7, 2022 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants