-
Notifications
You must be signed in to change notification settings - Fork 5
Add trait versioning #113
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
Add trait versioning #113
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Part of OpenAssetIO#80. The .editorconfig setting were only applied to `.yml` and not `.yaml`, yet our tests use `.yaml`. Signed-off-by: David Feltell <[email protected]>
Part of OpenAssetIO#80. The main OpenAssetIO project updated to Clang-Tidy 19 in OpenAssetIO/OpenAssetIO#1390. The TraitGen C++ tests run the generated code through `clang-tidy`, which failed for v19, specifically the `modernize-use-auto` check. So tweak the C++ code template to satisfy the failing check, allowing us to use a common Clang-Tidy version across projects. Signed-off-by: David Feltell <[email protected]>
Part of OpenAssetIO#80. The `unique_name_parts` field is not, in general, a tuple of a single element. In order to allow for arbitrary length tuples, we must suffix an ellipsis. Signed-off-by: David Feltell <[email protected]>
69b8b40 to
7280d76
Compare
7280d76 to
7d76d22
Compare
hutchinson
approved these changes
Oct 7, 2025
hutchinson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, i've limited experience with the innards of the code but nothing has jumped out as alarming!
Closes OpenAssetIO#80. Update the schema to support a dictionary of "versions" for traits and specifications, where each version contains a full trait/specification definition. Add a "deprecated" tag to the schema, allowing traits and specifications to be flagged as due for complete removal (i.e. all versions). Use string keys for the version, rather than integer keys, to provide maximum scope for future extension (e.g. tagged, semver). Note: this decision was not called out in the original design. Add a `.v1` suffix to trait IDs and an `_v1` suffix to trait and specification class names (where `1` is the version string). Do not add a version suffix to the ID for the first version of a trait, to maintain backward compatibility. Add trait classes that match the previous scheme, i.e. with no `_v1` suffix, which alias the first version of a trait, to maintain backward compatibility. Use inheritance to ensure maximum compatibility, rather than duplicating the class. Similarly to traits, add unversioned Specification view classes for backward compatibility. Non-versioned view classes are only for backward compatibility - going forward, hosts/managers should be explicit and use the version-suffixed classes. So mark non-versioned view classes as deprecated in their docstrings. For Python, add a `DeprecationWarning` using the `warnings` module when an unversioned view class is first constructed. Use the class name in the warning text, so a separate warning will be generated for each non-migrated class. For C++, add `[[deprecated]]` annotation to non-versioned view classes. Add deprecation docstrings and warnings for traits and specifications flagged with `deprecated: true`. Flag warnings at runtime in Python using the `warnings` module and C++ at compile time using the `[[deprecated]]` annotation. Update log output from command-line script to add version numbers. Since we're now using the `to_python_*_name` filters multiple times for the same input (in particular, class names), any names modified to be valid Python identifiers duplicate the same "Conforming" warning logs several times. So de-duplicate by keeping a cache of warnings that have already been logged. Add version specifiers to `TraitDeclaration`, `TraitReference` and `SpecificationDeclaration` for use in constructing IDs and class names. Add a CTest target for testing C++ deprecation warnings. Suppress at the compiler warning level, other than for the explicit deprecation warning test. Unfortunately, testing multiple raw string matches in build output via CTest is rather involved, but can be made to work. Signed-off-by: David Feltell <[email protected]>
7d76d22 to
aa5abef
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #80. Add support for versioning of traits and specifications, whilst retaining backward compatibility and adding deprecation warnings for old-style usage.
Implements the design discussed in OpenAssetIO/OpenAssetIO-MediaCreation#90
"<version number>": <definition>. This is the only breaking change._vXsuffix, whereXis a version number (e.g.LocatableContentTrait_v1)_vXsuffix..vXsuffix, except for the first version, which has no suffix for backward compatibility._vXsuffix, which are just an alias to_v1, but with deprecation warnings.deprecated=trueat the top-level of their YAML definition (i.e. above the versions).warningsmodule for Python and[[deprecated]]for C++, and@deprecatedin docstrings.See commits for more details.