Skip to content

Conversation

@durran
Copy link
Member

@durran durran commented Dec 9, 2021

Description

What is changing?

A new option was added to the db, client, operation and collection classes to allow users to enable/disable utf8 validation.

The legacy response class is not modified - deserialization changes are only present in the BinMsg class.

Is there new documentation needed for these changes?

Yes

What is the motivation for this change?

UTF8 validation is now enabled by default. Disabling utf8 validation can result in a performance benefit for users.

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

- tests are not passing, pushing to github to get feedback
- Validation options are not propagating to the binmsg layer for any configuration
- remove todo statement
- change test description
- rename test file to match code file name
@durran durran added the Team Review Needs review from team label Dec 9, 2021
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.

No changes required, but wait to merge until patch release

@dariakp dariakp added Blocked Blocked on other work and removed Blocked Blocked on other work labels Dec 9, 2021
@dariakp dariakp merged commit 4f56409 into main Dec 13, 2021
@dariakp dariakp deleted the NODE-3784/Add-utf8-validation-option branch December 13, 2021 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team Review Needs review from team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants