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

feat: 添加golangci-lint,并修复lint检查 #237

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

lbbniu
Copy link
Contributor

@lbbniu lbbniu commented Jun 3, 2023

No description provided.

@aceld
Copy link
Owner

aceld commented Jun 5, 2023

感谢提交PR!

@aceld
Copy link
Owner

aceld commented Jun 5, 2023

@lbbniu 你好,你是否确认要提交本地的一些yml文件?

@lbbniu
Copy link
Contributor Author

lbbniu commented Jun 6, 2023

确定

  • .golangci.yaml 是 golangci-lint 配置
  • reviewdog.yml Github Action 配置

Copy link
Owner

@aceld aceld left a comment

Choose a reason for hiding this comment

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

大部分格式修改已review

zinterceptor/framedocder.go Show resolved Hide resolved
zinterceptor/framedocder.go Show resolved Hide resolved
@aceld
Copy link
Owner

aceld commented Jun 7, 2023

@lbbniu 你好,大部分的格式修正已经review,非常的感谢,格式修正的也比较标准。

目前唯独一处代码修改存在逻辑问题

zinterceptor/framedocder.go

	//如果数据帧长度小于0,说明是个错误的数据包
	if frameLength < 0 {
		//内部会跳过这个数据包的字节数,并抛异常
		d.failOnNegativeLengthField(in, frameLength, d.LengthFieldEndOffset)
	}

495行左右的这段<0的逻辑是不可以去掉的。

getUnadjustedFrameLength()返回不能使用uint,因为长度有可能人为原因填写负数(也就是最高位为1),所以frameLength < 0是必要的。

辛苦再做调整,重新提交一次PR吧。

@lbbniu lbbniu force-pushed the feat/lbbniu/golangci-lint branch from 782074f to 7b96b31 Compare June 9, 2023 01:35
@lbbniu
Copy link
Contributor Author

lbbniu commented Jun 9, 2023

@aceld 冲突解决,相关代码已恢复

@lbbniu lbbniu requested a review from aceld June 9, 2023 01:39
@aceld
Copy link
Owner

aceld commented Jun 9, 2023

@lbbniu Thanks,已Merge.

@aceld aceld merged commit 37b1fe5 into aceld:master Jun 9, 2023
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.

2 participants