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.

Sign up for GitHub

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

Jump to bottom

Support go lint #395

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Support go lint #395

wants to merge 6 commits into from

Conversation

zhugelianglongming
Copy link
Collaborator

@zhugelianglongming zhugelianglongming commented Dec 27, 2022 •
edited
Loading

What changes

  • Support Go lint
  • Check Go lint before PR merge
  • Fix bugs
  • Remove redundant code
  • Fix typos
  • Deal with errors
  • Upgrade deprecated methods
  • Find failed tests

#378

How tested

  • Local make

@godeamon
Copy link
Collaborator

这个 pr 改的代码也太多了

@zhugelianglongming
Copy link
Collaborator Author

zhugelianglongming commented Dec 29, 2022 •
edited
Loading

这个 pr 改的代码也太多了

是的，但是没办法，开启报错之后，整个代码库都是错误。。。问题太多了。

代码库质量提升、集成自动校验肯定要经历这么一个过程，现在不改的话，以后也要改。除非要一直保留这些低质量代码。

这个代码库我改了非常久。

@zhugelianglongming
Copy link
Collaborator Author

zhugelianglongming commented Dec 29, 2022 •
edited
Loading

@godeamon 有一些错误的处理，我把握不太好，辛苦你 reivew 下。

这个代码库中有太多的错误选择性忽略，不显示处理的话，太危险了。

@godeamon
Copy link
Collaborator

如果一定要改的话，我建议你按照模块来慢慢改，一个是方便 review，另一个即使出问题了也好回滚。

@zhugelianglongming
Copy link
Collaborator Author

zhugelianglongming commented Dec 29, 2022 •
edited
Loading

如果一定要改的话，我建议你按照模块来慢慢改，一个是方便 review，另一个即使出问题了也好回滚。

这个建议好，我看看有没有拆分的办法。
拆分 PR:

@zhugelianglongming
Copy link
Collaborator Author

@godeamon 话说之前部分 patch 操作处理 error，部分不处理有什么讲究或者考虑在里面吗？

@zhugelianglongming zhugelianglongming mentioned this pull request Jan 4, 2023
Merged
@zhugelianglongming zhugelianglongming added the wontfix This will not be worked on label Jan 4, 2023
@zhugelianglongming zhugelianglongming mentioned this pull request Jan 4, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@godeamon godeamon Awaiting requested review from godeamon

@gaoweibupt gaoweibupt Awaiting requested review from gaoweibupt

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
wontfix This will not be worked on
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zhugelianglongming @godeamon