Drop legacy & internal registries from the public interface #625
Conversation
This file contains 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
They complicate the tests and maintenance of the framework. And they are very old, barely used, and exposed to the users by mistake (registries are not the framework's value).
The users should only use it as an object that is passed to the `registry=` option of the decorators and the `operator()` routines. They should never rely on any methods of accessing the content of the registry either for reading or for writing. By making these fields protected (underscore-named), we declare that, despite the fields are accessible, their naming, meaning, and own signatures can change any time without warning or ensuring backward compatibility. As part of this change, also hide those specialised sub-registries classes from the public interface of the framework. Originally, there was an idea to make a `OperatorRegistryAccessor` or `OperatorRegistryView` wrapper for internal use only. This would solve an issue with accessing the protected fields all over the code. But the severity of the issue is low: it only looks not nice, and might be caught by the linters (questionable) — while the accessor/view class would add complexity.
nolar
added
refactoring
nolar
changed the title
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.
Drop all legacy registries and hide all internal sub-registries from the public interface exposed to the users (operator developers).
THIS IS A BREAKING CHANGE. => Major version increase is required.
Background
These registries never were the main value of the framework, but a side-effect of a specific implementation in the past. The implementation changed several times, but the registries had to be maintained. Exposing them was a mistake in the first place. Their maintenance took more and more effort and would take even more unless they are made an internal implementation-specific detail.
The time has come to drop & hide them. Yet another refactoring of registries was needed as a prerequisite for a big refactoring of the reactor's core to be dynamic (#600), and this time, there was no way to ensure backward compatibility for the legacy registries.
Breaking changes
The users should only use
OperatorRegistryas an object that is passed to theregistry=option of the decorators and theoperator()routines. They should never rely on any methods of accessing the content of the registry either for reading or for writing.By making these fields protected (underscore-named), we declare that despite the fields are accessible, their naming, meaning, and own signatures can change any time without warning or ensuring backwards-compatibility.
Related: #511