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

cmake: add functions to define deprecated/experimental features #106

Merged
merged 1 commit into from
Nov 11, 2020

Conversation

andrea-iob
Copy link
Member

Experimental feature are disabled by default, whereas deprecated features are enabled by default.

Cmake variables related to experimental and deprecated functions are advanced variables and they will only be visible if the variables BITPIT_SHOW_EXPERIMENTAL or BITPIT_SHOW_DEPRECATED are set to ON.

For each deprecated/experimental feature a corresponding preprocessor macro will be defined. Given the feature FEATURE in the module MODULE, the corresponding preprocessor macro will be called BITPIT_MODULE_FEATURE. The value of the macro will be set to 1 if the feature is enable or it will be set to 0 if the feature is disabled.

Deprecated and experimental should be defined in the section "# Features" of the main CMakeLists.txt

@andrea-iob andrea-iob self-assigned this Oct 29, 2020
@andrea-iob andrea-iob marked this pull request as draft October 29, 2020 07:44
Copy link
Contributor

@alessandro-alaia alessandro-alaia left a comment

Choose a reason for hiding this comment

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

Nice 👍! I really like the idea of defining experimental/deprecated on a module/feature basis.
One problem I foresee: what if a deprecated/experimental feature depends on another deprecated/experimental feature. Is there a way to automatically deal with these "internal" dependencies? At the moment I don't think this should be a problem. In the next future, it might become cumbersome due to delay in deploying/removing of experimental/deprecated feature while other experimental features are being developed.

What I find a little confusing is the machinery of "first-showing-then-(de)activating". If I understand correctly you first need to enable "SHOW_deprecated/experimental" features/modules and then you can activate/de-activate such features. Any reason why activation/deactivation cannot be always exposed?

@andrea-iob
Copy link
Member Author

I didn't think about dependencies yet. We have some code to track the dependencies among modules, maybe we can borrow some of that code and use it to track feature dependencies.

I'm using the "SHOW_deprecated/experimental" variables as a way to make visible only deprecated or experimental features. There is no difference between variables created by defineDeprecatedModuleFeature and variable created by defineExperimentalModuleFeature (except for their initial state). Maybe be can simply merge the two functions together (defineModuleFeature) and remove the "SHOW_deprecated/experimental" variables.

@andrea-iob
Copy link
Member Author

I did some changes:

  • cmake variables "SHOW_DEPRECATED/EXPERIMENTAL" are gone;
  • the keyword DEPRECATED/EXPERIMENTAL is automatically added to both cmake variable and preprocess macro (e.g., the experimental feature FEATURE in the module MODULE will be associated with the cmake variable BITPIT_MODULE_EXPERIMENTAL_FEATURE).

@andrea-iob andrea-iob marked this pull request as ready for review November 9, 2020 10:44
Copy link
Contributor

@alessandro-alaia alessandro-alaia left a comment

Choose a reason for hiding this comment

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

Seems pretty clear to me and I don't see any potential issue. Merge is approved from my side.

Experimental feature are disabled by default, whereas deprecated
features are enabled by default.

Cmake variables related to experimental and deprecated functions are
advanced variables and they will only be visible if the variables
BITPIT_SHOW_EXPERIMENTAL or BITPIT_SHOW_DEPRECATED are set to ON.

For each deprecated/experimental feature a corresponding preprocessor
macro will be defined. Given the feature FEATURE in the module MODULE,
the corresponding preprocessor macro will be called BITPIT_MODULE_FEATURE.
The value of the macro will be set to 1 if the feature is enable or
it will be set to 0 if the feature is disabled.
@andrea-iob andrea-iob merged commit 7306366 into master Nov 11, 2020
@andrea-iob andrea-iob deleted the cmake.experimental.deprecated branch November 11, 2020 15:09
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.

2 participants