-
Notifications
You must be signed in to change notification settings - Fork 119
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
Introduce smallrye-config-api
module
#989
Conversation
implementation/src/main/java/io/smallrye/config/EnhancedConfig.java
Outdated
Show resolved
Hide resolved
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.
Looks OK to me, as long as we're sticking with methods that were already public and not adding more API. I don't like adding more API - the more API you have, the more "locked in" you are to a design, compatibility-wise.
Ok, how about if we keep it empty, and we add the methods we need as we go? Also, some time ago we discussed about having an |
3ae4400
to
2e940db
Compare
io.smallrye.config.Config
smallrye-config-api
module
2e940db
to
6e18a61
Compare
6e18a61
to
205696f
Compare
I'm not so sure about a whole separate module. I think that we are really due for a ground-up rethinking of how this project is put together and consumed. Many of the classes in the implementation module are in fact API, and this project has evolved far beyond the bounds of its original imagining. Maybe we should go over the list of classes and document approximately how they are used, and maybe try to categorize them, as a starting point. Or have a call where at least the three of us (or more?) talk it through "face to face" and figure out what ought to go where. WDYT? |
I agree. Probably better if we do the categorized work first, and then we can meet to discuss in detail. |
I went ahead with @dmlloyd's suggestion in #981 (comment) and created the
Config
interface.This avoids breaking existing code.
/cc @radcortez @dmlloyd