Skip to content

feat: support java.util.HashMap to es6 Map#70

Merged
fengmk2 merged 1 commit intomasterfrom
support-map
Aug 24, 2016
Merged

feat: support java.util.HashMap to es6 Map#70
fengmk2 merged 1 commit intomasterfrom
support-map

Conversation

@gxcsoccer
Copy link
Copy Markdown
Member

add $map property & set key, value into it while readHashMap

@mention-bot
Copy link
Copy Markdown

@gxcsoccer, thanks for your PR! By analyzing the annotation information on this pull request, we identified @fengmk2 and @dead-horse to be potential reviewers

Comment thread test/map.test.js
it('should decode successful when key is null', function () {
var data = new Buffer([77, 116, 0, 0, 78, 83, 0, 4, 110, 117, 108, 108, 122]);
var rv = hessian.decode(data);
rv.should.eql({null: 'null'});
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

breaking change, but i guess it's acceptable

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

可以接受

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

应该算 bugfix ,哈哈

@gxcsoccer
Copy link
Copy Markdown
Member Author

Array.from is not supported under node 4.x.

can we stop supporting node: < 4.0.0 ?

@codecov-io
Copy link
Copy Markdown

codecov-io commented Aug 24, 2016

Current coverage is 96.11% (diff: 100%)

Merging #70 into master will increase coverage by 0.06%

@@             master        #70   diff @@
==========================================
  Files             7          7          
  Lines          1063       1081    +18   
  Methods          95         95          
  Messages          0          0          
  Branches        197        203     +6   
==========================================
+ Hits           1021       1039    +18   
  Misses           42         42          
  Partials          0          0          

Powered by Codecov. Last update 2b53d13...9c1def9

Comment thread test/map.test.js Outdated
});

it('should read java.util.HashMap', function() {
if (!supportES6Map && Array.from) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这里是 or 关系吧

@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Aug 24, 2016

0.10 可以不支持,但是 0.12 还是要支持的,除非发 major 版本。

目前版本需要支持 0.12, 4, 6

@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Aug 24, 2016

master 刚刚合并了,需要 rebase 一下

@gxcsoccer gxcsoccer force-pushed the support-map branch 2 times, most recently from fdff031 to aeae33f Compare August 24, 2016 03:28
Comment thread lib/v1/decoder.js Outdated
// property name will auto transfer to a String type.
debug('read object prop: %j with type: %s', key, withType);
if (!/^this\$\d+$/.test(key)) {
if (!/^this\$\d+$/.test(key) && (typeof key === 'string' || typeof key === 'number')) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

key 是字符串才做这个正则判断吧

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok

Comment thread .travis.yml Outdated
@@ -7,7 +7,6 @@ node_js:
- '2'
- '1'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

只留 0.12, 4, 6, package.json engines 字段也改改

@gxcsoccer gxcsoccer force-pushed the support-map branch 2 times, most recently from 8d82bb3 to f94436a Compare August 24, 2016 04:17
@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Aug 24, 2016

+1

@fengmk2 fengmk2 merged commit a46fbc9 into master Aug 24, 2016
@fengmk2 fengmk2 deleted the support-map branch August 24, 2016 05:37
@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Aug 24, 2016

2.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants