The _detail and _internal namespace should be the last one in the chain of namespace following SDK design precedence

[ahsonkhan]

We currently have types in headers that are in the _detail namespace which contain sub-namespaces like Cryptography.

azure-sdk-for-cpp/sdk/tables/azure-data-tables/inc/azure/data/tables/internal/cryptography/url_encode.hpp

Line 12 in 0e699e4

namespace Azure { namespace Data { namespace Tables { namespace _detail { namespace Cryptography {

The existing precedence in the SDK is that _detail/_internal is the last component of the namespace.
See examples:

azure-sdk-for-cpp/sdk/core/azure-core/inc/azure/core/internal/cryptography/sha_hash.hpp

Line 18 in 0e699e4

namespace Azure { namespace Core { namespace Cryptography { namespace _internal {

azure-sdk-for-cpp/sdk/core/azure-core/inc/azure/core/internal/credentials/authorization_challenge_parser.hpp

Lines 15 to 16 in 0e699e4

	namespace Azure { namespace Core { namespace Credentials {
	namespace _detail {

cc @RickWinter

[LarryOsterman]

I have fairly strong feelings about this rule - the "_detail" and "_internal" being terminal namespaces can be incredibly limiting - it precludes the ability to express internal-only C++ namespaces - namespaces which should NEVER be exposed to customers.

For instance, this rule requires that the RustInterop namespace (which should NEVER be exposed to customers) appears as a direct child of Azure::Core::Amqp - and that implies that the RustInterop namespace is intended to contain public types (and top level public types as well). It would be incredibly easy to accidentally forget to put the types in that namespace in the _detail sub-namespace and thus accidentally expose those types.

But by putting the entire RustInterop namespace under a _detail namespace, it means that it is much harder to accidentally expose types in that namespace.

For both of the Core::Cryptography and Core::Credentials namespaces listed above, there ARE public APIs in the Credentials and Cryptography namespace. So in the case of namespaces which are supposed to contain public types, using a terminal _internal and _detail namespace makes complete sense.

But it doesn't necessarily make sense for Tables::Cryptography or Amqp::RustInterop, where there should never be any public types in the namespace.

[ahsonkhan]

That's a very interesting point. Thanks for sharing the concrete example motivating an exception where requiring _internal/_detail as a terminal namespace can pollute the set of public namespaces.

Would it be okay with you if I propose the wording in the C++ guidelines to capture the nuance of our stance, and drive this conversation to conclusion? I think we can carve out a reasonable exception to the rule.

[ahsonkhan]

FYI, the additional nuance you shared @LarryOsterman (for the exception cases) would need to be reconciled with our previous guidance when we designed and shipped v1:
#1789

The _detail namespace must be the last namespace in the namespace chain.

#### Detail namespaces
The detail namespace is implementation that is internal to the package (i.e. service).  These are implementation details and can change at any time without notice.  
DO NOT expose `::_detail` namespace to customers in public headers or return types.  Where possible the _detail namespaces should reside in private headers.  
Each package can define _detail namespace(s) as they see fit under one condition.  The _detail namespace must be the last namespace in the namespace chain.

Example:
`<service>::<somenamespace>::_detail` is valid

`<service>::_detail::<somenamespace>::` is invalid

[LarryOsterman]

I'd be interested in seeing that rule.

My personal version of it is (which is mostly consistent with previous guidance):

• Types which are not expected to be consumed outside the current package should contain the namespace "_detail" in the name.
• Types which are expected to be consumed by other packages in the Azure SDK should have "_internal" in their name.
• If a namespace contains public types, then _internal and _detail types must be in a namespace under the public namespace.
• Types which are intended to be consumed by clients outside the Azure SDK MUST NOT have "_internal" or "_detail" in their name.
• DO NOT Put types in the "_internal" namespace outside of the Azure::Core namespace.
  • One exception: Azure::Storage::Common and other "Common" packages.

The 3rd bullet encapsulates the current guidance, except for the final sentence (which I assert is excessively limiting).

Note that the final bullet generates an ApiView error if violated.

[RickWinter]

That's a very interesting point. Thanks for sharing the concrete example motivating an exception where requiring _internal/_detail as a terminal namespace can pollute the set of public namespaces.

Would it be okay with you if I propose the wording in the C++ guidelines to capture the nuance of our stance, and drive this conversation to conclusion? I think we can carve out a reasonable exception to the rule.

I can't recall why we settled on this being the "final" part of the namespace. It does seem overly restrictive.
Looping in @vhvb1989 in case he recalls context on why it had to be the last part of the namespace.

[antkmsft]

IIRC at a time we discussed them, there was no strong reason for _detail/_internal to be the terminal namespaces, we just decided to start with X::_detail until we find out we need _detail::X, and that is it.

We could at least say, until there is no Azure::Widget::X namespace, there can be Azure::Widget::_detail::X namespace. After that, it becomes confusing, whether it is X::_detail, or _detail::X, plus Azure::Widget::_detail::X would have to refer to Azure::Widget::X::Class as X::Class, while Azure::Widget::X::_detail does no need any namespace prefix - it could just refer to it as Class.

[vhvb1989]

Yeah, I remember looping thru this issue for a while and having everyone agree on the structure and valid usage of it.
It's mostly blurry to me now, but we did not have @LarryOsterman back then. It might've been a design choice between Rick and Jeff back then. But maybe a good time to re-validate?