You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This is the only case in the API package where internals are visible, and is confusing from the file structure point of view. It also allows for putting more complicated behavior into the helper, instead of changing the proper functions. (Like it happened in #49 (comment))
Options:
Make the corresponding methods public with an individual ApiStatus.Internal annotation on each one (The now deprecated WrappedConfig did this)
Move the wrapping relative stuff into an abstract ReflectiveConfigImpl which is extended by ReflectiveConfig
Use reflection to set the the inner value
My current favorite is option 2.
The text was updated successfully, but these errors were encountered:
Make the corresponding methods public with an individual ApiStatus.Internal annotation on each one (The now deprecated WrappedConfig did this)
Yeah, that change didn't happen on accident. The idea is that @ApiStatus.Internal on a random method provides barely any protection against mistaken use, while nobody is going to inadvertently use something called InternalsHelper.
Move the wrapping relative stuff into an abstract ReflectiveConfigImpl which is extended by ReflectiveConfig
Making an impl package class part of the class hierarchy of every ReflectiveConfig seems like a major downgrade from the current approach.
Use reflection to set the inner value
I do think using reflection to set the inner value would be fine; it's the ReflectiveConfig after all!
This is the only case in the API package where internals are visible, and is confusing from the file structure point of view. It also allows for putting more complicated behavior into the helper, instead of changing the proper functions. (Like it happened in #49 (comment))
Options:
ApiStatus.Internal
annotation on each one (The now deprecatedWrappedConfig
did this)ReflectiveConfigImpl
which is extended byReflectiveConfig
My current favorite is option 2.
The text was updated successfully, but these errors were encountered: