Skip to content
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

DefaultApplicationProvider 在加载/META-INF/app.properties 文件时代码优化 #2515

Merged
merged 2 commits into from
Aug 18, 2019

Conversation

kangjiabang
Copy link
Contributor

InputStream in = Thread.currentThread().getContextClassLoader().getResourceAsStream(APP_PROPERTIES_CLASSPATH); if (in == null) { in = DefaultApplicationProvider.class.getResourceAsStream(APP_PROPERTIES_CLASSPATH); }

DefaultApplicationProvider 在加载/META-INF/app.properties ,在使用contextClassLoader加载时,应该讲前缀"/"去除,否则加载到的结果为null,只能使用DefaultApplicationProvider.class.进行加载。

修改完成后,运行:
com.ctrip.framework.foundation.internals.provider.DefaultApplicationProviderTest#testLoadAppProperties ,结果OK

具体issue:
#2508

…ssLoader加载时,应该讲前缀"/"去除,否则加载到的结果为null,只能使用DefaultApplicationProvider进行加载
@codecov-io
Copy link

codecov-io commented Aug 12, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@dce5116). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2515   +/-   ##
=========================================
  Coverage          ?   50.19%           
  Complexity        ?     2046           
=========================================
  Files             ?      410           
  Lines             ?    12554           
  Branches          ?     1285           
=========================================
  Hits              ?     6301           
  Misses            ?     5806           
  Partials          ?      447
Impacted Files Coverage Δ Complexity Δ
...internals/provider/DefaultApplicationProvider.java 63.04% <100%> (ø) 11 <1> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dce5116...0c53c98. Read the comment docs.

@JaredTan95 JaredTan95 added this to the 1.5.0 milestone Aug 15, 2019
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 53.752% when pulling 0c53c98 on kangjiabang:master into dce5116 on ctripcorp:master.

@kangjiabang
Copy link
Contributor Author

@JaredTan95 ,please review

Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nobodyiam nobodyiam merged commit 5fa93b6 into apolloconfig:master Aug 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants