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

Rename IMATH_IMATH_NAMESPACE option to IMATH_NAMESPACE #111

Merged
merged 2 commits into from
Mar 13, 2021

Conversation

cary-ilm
Copy link
Member

@cary-ilm cary-ilm commented Mar 8, 2021

Also, replace IMATH_INTERNAL_IMATH_NAMESPACE to IMATH_INTERNAL_NAMESPACE.

These were likely originally derived from ILMBASE_IMATH_NAMESPACE, which made sense, but the double IMATH seems confusing and unnecessary.

Note this fixes the CMake options, not the #define symbols, which already have the proper values. This change makes the two consistent.

Signed-off-by: Cary Phillips cary@ilm.com

Also, replace IMATH_INTERNAL_IMATH_NAMESPACE to IMATH_INTERNAL_NAMESPACE.

These were likely originally derived from ILMBASE_IMATH_NAMESPACE, which made sense, but the double IMATH seems confusing and unnecessary.

Note this fixes the CMake options, not the #define symbols, which already have the proper values. This change makes the two consistent.

Signed-off-by: Cary Phillips <cary@ilm.com>
@lgritz
Copy link
Contributor

lgritz commented Mar 8, 2021

I wonder if we should simplify by switching from a 2-deep nested namespace, both of which are customized, to a single (customizable) namespace and an alias.

We would still define

set(IMATH_NAMESPACE "Imath_${IMATH_VERSION_API}" CACHE STRING)

but then the namespace declaration would be

namespace IMATH_NAMESPACE {  };
using namespace Imath = IMATH_NAMESPACE;

and that's it.

This would be literally the only place in the whole codebase where IMATH_NAMESPACE is used, and users would never need to use it. Both all of our internals and all user could would always say Imath::blah, and the resulting symbols would be automatically versioned and optionally changed to anything else the builder customized.

Is there really a reason why it's still useful to implement the versioning and customization as (a) two-level, and (b) having a potentially custom user-referring name as well as a separate custom symbol-mangling name?

@meshula
Copy link
Contributor

meshula commented Mar 8, 2021

I like updating the scheme to use a language feature that didn't exist when the original scheme was invented.

@cary-ilm
Copy link
Member Author

cary-ilm commented Mar 9, 2021 via email

@cary-ilm
Copy link
Member Author

Can someone sign off on this? I believe it's a rather innocuous correction of a CMake configuration setting whose name got mangled in the port from IlmBase.

@cary-ilm cary-ilm merged commit 4c53e0f into AcademySoftwareFoundation:master Mar 13, 2021
@cary-ilm cary-ilm added v3.0.0-beta documentation Improvements or additions to documentation build labels Mar 17, 2021
@cary-ilm cary-ilm deleted the cmake-namespace branch May 7, 2021 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build documentation Improvements or additions to documentation v3.0.0-beta
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants