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

Code format #199

Merged
merged 7 commits into from
Jun 10, 2020
Merged

Code format #199

merged 7 commits into from
Jun 10, 2020

Conversation

gaoxinge
Copy link
Contributor

@gaoxinge gaoxinge commented Jun 5, 2020

What this PR does:

  • clean code

Special notes for your reviewer:

  • NONE

Does this PR introduce a user-facing change?:

NONE

@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2020

Codecov Report

Merging #199 into master will increase coverage by 0.02%.
The diff coverage is 87.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #199      +/-   ##
==========================================
+ Coverage   66.39%   66.41%   +0.02%     
==========================================
  Files          22       22              
  Lines        2845     2835      -10     
==========================================
- Hits         1889     1883       -6     
+ Misses        740      738       -2     
+ Partials      216      214       -2     
Impacted Files Coverage Δ
string.go 57.91% <87.50%> (-0.07%) ⬇️

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 f7b44b5...7c66578. Read the comment docs.

@wongoo
Copy link
Contributor

wongoo commented Jun 5, 2020

@gaoxinge can u add some comment for the change pls?

@gaoxinge
Copy link
Contributor Author

gaoxinge commented Jun 8, 2020

@wongoo @zonghaishang @AlexStocks

  • 移除
if chunkLen < 0 {
    chunkLen = 0
}

这段条件赋值是因为这段代码是在chunkLen <= 0的前提下执行的:

if chunkLen <= 0 {

所以这段代码等价于chunkLen = 0

  • 移除
if charLen < 0 {
    charLen = 0
}

是因为charLen是通过getStringLength获得的,因此我倾向于在getStringLength里面检查charLen的合法性,并以报错的形式呈现。

  • chunkLen += charLen改成chunkLen = charLen是因为chunkLen = 0一定成立,所以把原地加法改成了赋值。

@zonghaishang
Copy link
Member

如果读取的chunkLen是负数怎么办? err也不是nil ?

@gaoxinge
Copy link
Contributor Author

gaoxinge commented Jun 8, 2020

@zonghaishang

getStringLength里面添加了字符串长度的检查:

dubbo-go-hessian2/string.go

Lines 252 to 254 in ca016b0

if length < 0 {
return -1, perrors.Errorf("string decode: length less than zero")
}

string.go Show resolved Hide resolved
@gaoxinge
Copy link
Contributor Author

gaoxinge commented Jun 9, 2020

@zonghaishang @wongoo @AlexStocks 经过多次讨论,还是把检查字符串长度的代码逻辑去掉了 。因为理论上hessian保证了解析出来的长度一定大于等于0的。如果返回了小于0的值,那只可能是getStringLength有bug。

Copy link
Contributor

@wongoo wongoo left a comment

Choose a reason for hiding this comment

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

LGTM

@wongoo wongoo merged commit 607925d into apache:master Jun 10, 2020
@gaoxinge gaoxinge deleted the code_format branch June 10, 2020 05:21
wongoo added a commit that referenced this pull request Jun 15, 2020
* add license checker (#175)

* add release note for v1.5.0 (#178)

Co-authored-by: Xin.Zh <dragoncharlie@foxmail.com>

* Imp: cache in reflection (#179)

* benchmark result

* use cache in findField

* encode benchmark

* call field() once

* remove version

* fix import sync

* cache in registerPOJO

* add json bench result

* prune unneccessary rtype.Field(index)

* cache comment

* rename cache

* switch to if

* remove return value name

* findFieldWithCache

* remove if check when fieldStruct is nil

Co-authored-by: 望哥 <gelnyang@163.com>

* update dependency

* rename serialize arg name

* Create .asf.yaml

* 优化hessian解码string性能,提升54%

* optimize code.

* optimize code.

* fix code review.

* optimize codes.

* optimize cods.

* optimize code.

* update license

* go.sum

* ci go version

* testify -> 1.4.0

* testcase

* travis.yml

* decode value before reflect find

* setvalue

* decode nilPtr to nilPtr

* fix get attachment lost nil key

* manually import package

* add ToMapStringString unit test

* rename test function name with issue

* setmap

* support for decode emoji.

* refactor code

* add unit test.

* add unit tests.

* refactor tests.

* Update travis/main.sh (#200)

- Remove duplicate key 'webhooks'
- Key 'matrix' is an alias for `jobs`, using `jobs`
- Specify the os and dist explicitly

* Mod: modify

* Code format (#199)

* .gitignore

* code clean

* code clean

* remove length check

* Fix: comments

* Fix: format package

* Fix #181: float32 accuracy issue (#196)

* Fix #181: float32 accuracy issue

* Fix go fmt failure

* Add the unit test case for Issue181

* Add encFloat32 in double.go to encode float32 type

- Call encFloat32 to encode float32 while encoding
- Add unit test case to test float32 encoding

* Improve encFloat32 of double.go

* Fix git fmt failure

* add release note for v1.6.0 (#202)

* add release note for v1.5.1

* add release note for v1.5.1

* add notice

* update notice

* =fix release note for v1.6.0

Co-authored-by: Joe Zou <joezou@apache.org>
Co-authored-by: Xin.Zh <dragoncharlie@foxmail.com>
Co-authored-by: huiren <zhrlnt@gmail.com>
Co-authored-by: Huang YunKun <htynkn@gmail.com>
Co-authored-by: zonghaishang <yiji@apache.org>
Co-authored-by: fangyincheng <fangyc666@gmail.com>
Co-authored-by: champly <champly@outlook.com>
Co-authored-by: wilson chen <willson.chenwx@gmail.com>
Co-authored-by: fangyincheng <fangyincheng@sina.com>
Co-authored-by: gaoxinge <gaoxx5@gmail.com>
wongoo added a commit that referenced this pull request Jun 23, 2020
* add license checker (#175)

* add release note for v1.5.0 (#178)

Co-authored-by: Xin.Zh <dragoncharlie@foxmail.com>

* Imp: cache in reflection (#179)

* benchmark result

* use cache in findField

* encode benchmark

* call field() once

* remove version

* fix import sync

* cache in registerPOJO

* add json bench result

* prune unneccessary rtype.Field(index)

* cache comment

* rename cache

* switch to if

* remove return value name

* findFieldWithCache

* remove if check when fieldStruct is nil

Co-authored-by: 望哥 <gelnyang@163.com>

* update dependency

* rename serialize arg name

* Create .asf.yaml

* 优化hessian解码string性能,提升54%

* optimize code.

* optimize code.

* fix code review.

* optimize codes.

* optimize cods.

* optimize code.

* update license

* go.sum

* ci go version

* testify -> 1.4.0

* testcase

* travis.yml

* decode value before reflect find

* setvalue

* decode nilPtr to nilPtr

* fix get attachment lost nil key

* manually import package

* add ToMapStringString unit test

* rename test function name with issue

* setmap

* support for decode emoji.

* refactor code

* add unit test.

* add unit tests.

* refactor tests.

* Update travis/main.sh (#200)

- Remove duplicate key 'webhooks'
- Key 'matrix' is an alias for `jobs`, using `jobs`
- Specify the os and dist explicitly

* Mod: modify

* Code format (#199)

* .gitignore

* code clean

* code clean

* remove length check

* Fix: comments

* Fix: format package

* Fix #181: float32 accuracy issue (#196)

* Fix #181: float32 accuracy issue

* Fix go fmt failure

* Add the unit test case for Issue181

* Add encFloat32 in double.go to encode float32 type

- Call encFloat32 to encode float32 while encoding
- Add unit test case to test float32 encoding

* Improve encFloat32 of double.go

* Fix git fmt failure

* add release note for v1.6.0 (#202)

* add release note for v1.5.1

* add release note for v1.5.1

* add notice

* update notice

* =fix release note for v1.6.0

* Fix: eunm encode in request get error (#203)

* fix bug: eunm encode in request get error

* fix mod

* add ut

* remove go.mod 1.13 to fix the ci

Co-authored-by: Joe Zou <joezou@apache.org>
Co-authored-by: Xin.Zh <dragoncharlie@foxmail.com>
Co-authored-by: huiren <zhrlnt@gmail.com>
Co-authored-by: Huang YunKun <htynkn@gmail.com>
Co-authored-by: zonghaishang <yiji@apache.org>
Co-authored-by: fangyincheng <fangyc666@gmail.com>
Co-authored-by: champly <champly@outlook.com>
Co-authored-by: wilson chen <willson.chenwx@gmail.com>
Co-authored-by: fangyincheng <fangyincheng@sina.com>
Co-authored-by: gaoxinge <gaoxx5@gmail.com>
Co-authored-by: panty <pantianying@gmail.com>
zhaoyunxing92 pushed a commit that referenced this pull request Sep 4, 2021
* .gitignore

* code clean

* code clean

* remove length check
zhaoyunxing92 pushed a commit that referenced this pull request Sep 4, 2021
* add license checker (#175)

* add release note for v1.5.0 (#178)

Co-authored-by: Xin.Zh <dragoncharlie@foxmail.com>

* Imp: cache in reflection (#179)

* benchmark result

* use cache in findField

* encode benchmark

* call field() once

* remove version

* fix import sync

* cache in registerPOJO

* add json bench result

* prune unneccessary rtype.Field(index)

* cache comment

* rename cache

* switch to if

* remove return value name

* findFieldWithCache

* remove if check when fieldStruct is nil

Co-authored-by: 望哥 <gelnyang@163.com>

* update dependency

* rename serialize arg name

* Create .asf.yaml

* 优化hessian解码string性能,提升54%

* optimize code.

* optimize code.

* fix code review.

* optimize codes.

* optimize cods.

* optimize code.

* update license

* go.sum

* ci go version

* testify -> 1.4.0

* testcase

* travis.yml

* decode value before reflect find

* setvalue

* decode nilPtr to nilPtr

* fix get attachment lost nil key

* manually import package

* add ToMapStringString unit test

* rename test function name with issue

* setmap

* support for decode emoji.

* refactor code

* add unit test.

* add unit tests.

* refactor tests.

* Update travis/main.sh (#200)

- Remove duplicate key 'webhooks'
- Key 'matrix' is an alias for `jobs`, using `jobs`
- Specify the os and dist explicitly

* Mod: modify

* Code format (#199)

* .gitignore

* code clean

* code clean

* remove length check

* Fix: comments

* Fix: format package

* Fix #181: float32 accuracy issue (#196)

* Fix #181: float32 accuracy issue

* Fix go fmt failure

* Add the unit test case for Issue181

* Add encFloat32 in double.go to encode float32 type

- Call encFloat32 to encode float32 while encoding
- Add unit test case to test float32 encoding

* Improve encFloat32 of double.go

* Fix git fmt failure

* add release note for v1.6.0 (#202)

* add release note for v1.5.1

* add release note for v1.5.1

* add notice

* update notice

* =fix release note for v1.6.0

Co-authored-by: Joe Zou <joezou@apache.org>
Co-authored-by: Xin.Zh <dragoncharlie@foxmail.com>
Co-authored-by: huiren <zhrlnt@gmail.com>
Co-authored-by: Huang YunKun <htynkn@gmail.com>
Co-authored-by: zonghaishang <yiji@apache.org>
Co-authored-by: fangyincheng <fangyc666@gmail.com>
Co-authored-by: champly <champly@outlook.com>
Co-authored-by: wilson chen <willson.chenwx@gmail.com>
Co-authored-by: fangyincheng <fangyincheng@sina.com>
Co-authored-by: gaoxinge <gaoxx5@gmail.com>
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