-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
ConfigFileFormat#Properties
are now fully compatible with themselves
#4033
ConfigFileFormat#Properties
are now fully compatible with themselves
#4033
Conversation
Previously `ConfigFileFormat#Properties` were not fully compatible by themself which meant the function `ConfigFileFormat.isPropertiesCompatible` was not really meaning full. PropertiesConfigFiles are now designed to be compatible bythemselves by implementing `PropertiesCompatibleConfigFile`. This might be usefull in further development. Currently `DefaultConfigFactory.create` will not create a properties compatible repository since this is not really usefull and a localRepositry might be easier to use in further development.
Codecov Report
@@ Coverage Diff @@
## master #4033 +/- ##
============================================
+ Coverage 51.63% 51.65% +0.01%
- Complexity 2543 2547 +4
============================================
Files 484 484
Lines 14839 14844 +5
Branches 1536 1536
============================================
+ Hits 7662 7667 +5
Misses 6645 6645
Partials 532 532
Continue to review full report at Codecov.
|
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.
Thanks for your contribution! Here are some minor comments:
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PropertiesConfigFile.java
Outdated
Show resolved
Hide resolved
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java
Show resolved
Hide resolved
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java
Outdated
Show resolved
Hide resolved
…goKrupitza/apollo into chore/configFileFormat-compatibility
@nobodyiam everything should be resolved now 👍🏽😌 |
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.
LGTM
Previously
ConfigFileFormat#Properties
were not fully compatible bythemself which meant the function
ConfigFileFormat.isPropertiesCompatible
was not really meaning full. PropertiesConfigFiles are now designed to be
compatible bythemselves by implementing
PropertiesCompatibleConfigFile
.This might be usefull in further development.
Currently
DefaultConfigFactory.create
will not create a propertiescompatible repository since this is not really usefull and a
localRepositry might be easier to use in further development.
What's the purpose of this PR
In PR #4030 we found out that
ConfigFileFormat#Properties
are not compatible by themselves which might become problematic in the future.Which issue(s) this PR fixes:
None
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean test
to make sure this pull request doesn't break anything.CHANGES
log.