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

Fix #1411 Java Locale use '_' split language, country, variant. #1413

Merged
merged 1 commit into from Apr 4, 2018

Conversation

takeseem
Copy link
Contributor

@takeseem takeseem commented Mar 1, 2018

see #1411

Java Locale.toString() split language, country and variant with char \_.

but not support the extentions, because of truncate \_#.* in the commit.

@CLAassistant
Copy link

CLAassistant commented Mar 9, 2018

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
4 out of 5 committers have signed the CLA.

✅ takeseem
✅ ma-xiao-guang-64
✅ beiwei30
✅ chickenlj
❌ 杨浩


杨浩 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@takeseem takeseem reopened this Mar 9, 2018
private String language;
private String country;
private String variant;
private String value;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

revert commit 0423219

线上环境需要LocaleHandler版本兼容,所以还原代码。

@@ -68,7 +68,7 @@ public void writeObject(Object obj, AbstractHessianOutput out)
else {
Locale locale = (Locale) obj;

out.writeObject(new LocaleHandle(locale.getLanguage(), locale.getCountry(), locale.getVariant()));
out.writeObject(new LocaleHandle(locale.toString()));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

revert commit 0423219

线上环境需要LocaleHandler版本兼容,所以还原代码。

@codecov-io
Copy link

codecov-io commented Mar 9, 2018

Codecov Report

Merging #1413 into master will increase coverage by 0.01%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1413      +/-   ##
=========================================
+ Coverage   31.59%   31.6%   +0.01%     
=========================================
  Files         682     682              
  Lines       33035   33048      +13     
  Branches     6592    6597       +5     
=========================================
+ Hits        10436   10446      +10     
- Misses      20748   20750       +2     
- Partials     1851    1852       +1
Impacted Files Coverage Δ
...libaba/com/caucho/hessian/io/LocaleSerializer.java 75% <100%> (ø) ⬆️
...om/alibaba/com/caucho/hessian/io/LocaleHandle.java 84.21% <82.35%> (-15.79%) ⬇️
...om/alibaba/dubbo/rpc/filter/ActiveLimitFilter.java 83.33% <0%> (-5.56%) ⬇️
...a/dubbo/remoting/transport/netty/NettyChannel.java 61.25% <0%> (-5%) ⬇️
...ubbo/rpc/protocol/dubbo/ChannelWrappedInvoker.java 41.66% <0%> (-4.17%) ⬇️
...om/alibaba/com/caucho/hessian/io/HessianInput.java 13.19% <0%> (+0.17%) ⬆️
...baba/dubbo/remoting/transport/mina/MinaClient.java 59.42% <0%> (+1.44%) ⬆️
...bo/remoting/transport/netty/NettyCodecAdapter.java 56.71% <0%> (+1.49%) ⬆️
...m/alibaba/com/caucho/hessian/io/HessianOutput.java 18.7% <0%> (+1.79%) ⬆️

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 2308427...f7572ab. Read the comment docs.

@diecui1202
Copy link

@beiwei30 please take some time to review this PR. I find it reverts your previous commit 0423219

@beiwei30 beiwei30 merged commit 71e04fb into apache:master Apr 4, 2018
@beiwei30
Copy link
Member

beiwei30 commented Apr 4, 2018

let's merge this change, and consider how to improve it better. it doesn't have major impact anyway.

@chickenlj chickenlj added this to the 2.6.2 milestone Apr 24, 2018
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.

None yet

6 participants