Skip to content

CDRIVER-3985 Remove Oppressive Terminology From Code #794

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

Merged
merged 13 commits into from
May 26, 2021

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented May 19, 2021

CDRIVER-3985

This PR deprecates and removes oppressive terminology from code. The spec test changes will be synced one more time once mongodb/specifications#990 is merged.

The last two commits commit are the changes from #793 and #792 to keep the build somewhat happy, so I would recommend looking at the changes commit by commit.

@alcaeus alcaeus requested review from kevinAlbs and benjirewis May 19, 2021 12:11
@alcaeus alcaeus self-assigned this May 19, 2021
Copy link
Contributor

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

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

Looking good!

@alcaeus alcaeus force-pushed the cdriver-3985 branch 2 times, most recently from 7400721 to 0c60837 Compare May 21, 2021 07:58
@alcaeus alcaeus requested a review from benjirewis May 21, 2021 07:58
Copy link
Contributor

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

Looking great!

I ran the grep command that James included in the comment of CDRIVER-3887, I think there are a handful of docs that need updating (the other "not master" changes in code I think will be part of CDRIVER-3997):

$ find . -iname "*.rst" | xargs ggrep -PriI '(docs\.mongodb\.com\/master|(?<!/)master(?!.?key)|slave|whitelist|blacklist)' 
./libmongoc/tests/json/retryable_reads/README.rst:waiting time after a "not master" error:
./libmongoc/doc/installing.rst:Clone the repository and prepare the build on the current master branch or a particular release tag:
./libmongoc/doc/mongoc_server_description_t.rst:    mongoc_server_description_ismaster
./libmongoc/doc/mongoc_collection_update_many.rst:  | `MongoDB update command documentation <https://docs.mongodb.com/master/reference/command/update/>`_ for more information on the update options.
./libmongoc/doc/mongoc_collection_update_one.rst:  | `MongoDB update command documentation <https://docs.mongodb.com/master/reference/command/update/>`_ for more information on the update options.
./libmongoc/doc/mongoc_server_description_hello_response.rst:`"hello" <https://docs.mongodb.org/manual/reference/command/isMaster/>`_
./libmongoc/doc/mongoc_read_concern_t.rst:See `Read Concern Levels <https://docs.mongodb.com/master/reference/read-concern/#read-concern-levels>`_ in the MongoDB manual for more information about the individual read concern levels.
./libmongoc/doc/mongoc_client_set_appname.rst:Sets the application name for this client. This string, along with other internal driver details, is sent to the server as part of the initial connection handshake (`"hello" <https://docs.mongodb.org/manual/reference/command/isMaster/>`_).
./libmongoc/doc/mongoc_collection_replace_one.rst:  | `MongoDB update command documentation <https://docs.mongodb.com/master/reference/command/update/>`_ for more information on the update options.

I think docs links can be updated:

  • master => manual.
  • https://docs.mongodb.com/manual/reference/command/isMaster/ => https://docs.mongodb.com/manual/reference/command/hello/

@@ -13,7 +13,7 @@ Synopsis
typedef enum {
MONGOC_QUERY_NONE = 0,
MONGOC_QUERY_TAILABLE_CURSOR = 1 << 1,
MONGOC_QUERY_SLAVE_OK = 1 << 2,
MONGOC_QUERY_SECONDARY_OK = 1 << 2,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good thinking. I originally thought you removed MONGOC_QUERY_SLAVE_OK entirely. But leaving it out of documentation seems like a good idea.

I filed CDRIVER-4006 to track removing public symbols in the next major release.


mongoc_server_description_ismaster()
mongoc_server_description_hello()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This may have been an unintentional change from find/replace. Though it's better to document deprecated symbols, existing users of mongoc_server_description_ismaster will get a warning during upgrade about the because of BSON_GNUC_DEPRECATED_FOR (mongoc_server_description_hello_response).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this was an unintended change, good catch! I've reverted this to the legacy naming; documentation for the deprecated function should stay in place until the function itself is removed.

@alcaeus
Copy link
Member Author

alcaeus commented May 25, 2021

Yes, more changes are incoming for CDRIVER-3997, which should also take care of the last tests. I've removed the other instances shown in your comment.

@alcaeus alcaeus requested a review from kevinAlbs May 25, 2021 12:57
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

LGTM mod one minor comment:

  • README.rst has "- Examples: master branch, 1.9.5 tag". I don't think the branch helps, so I think this can be just "- Examples: 1.9.5 tag"

@alcaeus alcaeus merged commit f1093ba into mongodb:master May 26, 2021
@alcaeus alcaeus deleted the cdriver-3985 branch May 26, 2021 18:00
kevinAlbs pushed a commit that referenced this pull request Jun 3, 2021
* Remove deprecated terminology for legacy hello

* Remove deprecated terminology for secondaryOk

* Remove deprecated terminology for allow/deny list

* Remove deprecated terminology for notPrimary

* Update terminology in spec tests

* Fix issues from code review

* Remove more language in docs

* Undo accidental change to hello

* Use secondaryOk when printing query flags

* Fix wrong variable name

* Remove deprecated terminology in tests

* Remove branch name from installation docs

* Clarify build examples in readme
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