Skip to content

Conversation

ljhaywar
Copy link
Contributor

@ljhaywar ljhaywar commented Dec 2, 2021

Description

NODE-3788

What is changing?

  • Error codes with offensive names are being renamed
  • Other offensive wording in error.ts is being renamed

The tests that reference these error codes are spec tests. Updates to those tests are covered in NODE-3791.

Is there new documentation needed for these changes?

No

What is the motivation for this change?

Following the guidance documented in https://tools.ietf.org/id/draft-knodel-terminology-00.html, we will remove all oppressive and unnecessarily gendered language in driver documentation, code, tests, specs, and spec tests.

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

@ljhaywar ljhaywar added the wip label Dec 2, 2021
@ljhaywar ljhaywar self-assigned this Dec 2, 2021
@ljhaywar
Copy link
Contributor Author

ljhaywar commented Dec 2, 2021

Questions:

  • I renamed a constant and a function. Neither are exported, so I think that means I can make the changes without any deprecation notices. Is that the case?
  • I updated the return message for two functions. From what I can tell, those messages are only being used inside of this file. Are there any other places the messages are being used? Is it OK to change the error message (is anyone relying on that specific error message)?

@ljhaywar ljhaywar changed the title Update references to master in error.ts refactor(NODE-3788):Update names of offensive error codes Dec 2, 2021
@ljhaywar ljhaywar removed the wip label Dec 3, 2021
@ljhaywar ljhaywar marked this pull request as ready for review December 3, 2021 13:04
@baileympearson
Copy link
Contributor

Questions:

  • I renamed a constant and a function. Neither are exported, so I think that means I can make the changes without any deprecation notices. Is that the case?
  • I updated the return message for two functions. From what I can tell, those messages are only being used inside of this file. Are there any other places the messages are being used? Is it OK to change the error message (is anyone relying on that specific error message)?
  • Yup, renaming the constant and the function should be fine. It's a purely internal change and has no functionality change.
  • The return messages you updated are actually only returning a boolean. Those functions are testing an error message against the strings up updated to determine if the error message passed in to the function matches those error strings. I think this is the scenario Daria mentioned and is not safe to update, since these error messages are being returned from the server.

durran
durran previously requested changes Dec 3, 2021
@dariakp dariakp added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Dec 3, 2021
@ljhaywar
Copy link
Contributor Author

ljhaywar commented Dec 3, 2021

We're seeing the same test failures in more than one PR, so I don't think these failures are related to the code changes.

@baileympearson this is ready for your review

@baileympearson baileympearson added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Dec 3, 2021
@ljhaywar ljhaywar force-pushed the NODE-3788/update-names-of-offensive-error-codes branch from ffac9f6 to 3875192 Compare December 6, 2021 15:21
@baileympearson baileympearson requested a review from durran December 6, 2021 16:49
@baileympearson baileympearson dismissed durran’s stale review December 6, 2021 16:50

Comments were addressed

@nbbeeken nbbeeken changed the title refactor(NODE-3788):Update names of offensive error codes refactor(NODE-3788): update names of offensive error codes Dec 6, 2021
@baileympearson baileympearson merged commit 47c7893 into main Dec 6, 2021
@baileympearson baileympearson deleted the NODE-3788/update-names-of-offensive-error-codes branch December 6, 2021 19:17
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.

5 participants