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

doc: remove inline namespace from doxygen #7461

Merged
merged 2 commits into from
Oct 14, 2021
Merged

Conversation

devjgm
Copy link
Contributor

@devjgm devjgm commented Oct 14, 2021

Fixes: #7453

With this change, our Doxygen documentation will no longer make any
mention of our inline namespace for versioning, thus the docs will be
accurately reflecting what we actually want customers to type.

This PR removes all references to omit_this_inline_ns.


This change is Reviewable

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 14, 2021
@google-cloud-cpp-bot
Copy link
Collaborator

Google Cloud Build Logs
For commit: d9c940c7c0f60ecf269b79a150146ccc57737db0

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

Fixes: googleapis#7453

With this change, our Doxygen documentation will no longer make any
mention of our inline namespace for versioning, thus the docs will be
accurately reflecting what we actually want customers to type.

This PR removes all references to `omit_this_inline_ns`.
@devjgm devjgm marked this pull request as ready for review October 14, 2021 11:29
@devjgm devjgm requested a review from a team as a code owner October 14, 2021 11:29
@google-cloud-cpp-bot
Copy link
Collaborator

Google Cloud Build Logs
For commit: 64751ac1e81fe902b164e9a4b63ea35c6b51cccb

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

@codecov
Copy link

codecov bot commented Oct 14, 2021

Codecov Report

Merging #7461 (12d90fe) into main (c0cb1c5) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #7461   +/-   ##
=======================================
  Coverage   93.64%   93.64%           
=======================================
  Files        1364     1364           
  Lines      118362   118362           
=======================================
+ Hits       110842   110844    +2     
+ Misses       7520     7518    -2     
Impacted Files Coverage Δ
google/cloud/bigtable/instance_admin.h 100.00% <ø> (ø)
google/cloud/bigtable/table.h 100.00% <ø> (ø)
google/cloud/bigtable/table_admin.h 85.48% <ø> (ø)
google/cloud/pubsub/mocks/mock_ack_handler.h 100.00% <ø> (ø)
...gle/cloud/pubsub/mocks/mock_publisher_connection.h 100.00% <ø> (ø)
.../cloud/pubsub/mocks/mock_schema_admin_connection.h 100.00% <ø> (ø)
...le/cloud/pubsub/mocks/mock_subscriber_connection.h 100.00% <ø> (ø)
.../pubsub/mocks/mock_subscription_admin_connection.h 100.00% <ø> (ø)
...e/cloud/pubsub/mocks/mock_topic_admin_connection.h 100.00% <ø> (ø)
google/cloud/pubsub/publisher.h 100.00% <ø> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c0cb1c5...12d90fe. Read the comment docs.

Copy link
Contributor

@coryan coryan left a comment

Choose a reason for hiding this comment

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

I think we will want to spend some time fine tuning the CHANGELOG. Maybe merge as-is and we can reword the CHANGELOG in separate PRs?

CHANGELOG.md Outdated
@@ -69,6 +69,22 @@

## v1.33.0 - TBD

**ATTENTION**: Users should generally **NOT** spell the name of our versioned
Copy link
Contributor

Choose a reason for hiding this comment

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

This paragraph assumes that one knows what "our versioned inline namespace" is. And it is a bit informal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

CHANGELOG.md Outdated
specific version of our library and will make upgrades more difficult for you.
Previously, this version was `v1` (but it will change in the future), and so
you may have some code that references, say, ~`google::cloud::v1::Status`~
(WRONG) and you should instead prefer `google::cloud::Status` (omit the
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't yell at the customers. Specially if it was our fault. Maybe just say:

For example, prefer using `google::cloud::Status` in your code, over `google::cloud::v1::Status`, as the latter makes it harder to update your code when upgrading the libraries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

CHANGELOG.md Outdated
(WRONG) and you should instead prefer `google::cloud::Status` (omit the
versioned inline namespace name).

Our Doxygen documentation (e.g. [Storage docs][storage-dox-link]) was
Copy link
Contributor

Choose a reason for hiding this comment

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

I would lead with this paragraph.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@google-cloud-cpp-bot
Copy link
Collaborator

Google Cloud Build Logs
For commit: 12d90fe944dec5cbc03f46e287fb2de62cad5fdc

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

@devjgm devjgm merged commit cc0291e into googleapis:main Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN (and _END) to declare inline namespace
4 participants