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

chore: use errors.New to replace fmt.Errorf with no parameters #12769

Closed
wants to merge 1 commit into from

Conversation

ChengenH
Copy link
Contributor

  1. use errors.New to replace fmt.Errorf with no parameters
  2. Fix spelling errors in words

Signed-off-by: ChengenH <hce19970702@gmail.com>
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. As nice as this is, it's not a hard standard that we enforce in this codebase, in fact we use xerrors more than errors, but even that's not consistent except where we're wrapping errors.

This PR just opens the door to a larger change-set that I don't think I want to encourage as small PRs because it'll just be highlighting the inconsistency rather than solving it:

$ git grep -E 'fmt\.Errorf\("[^%]*"\)' -- "*.go" | wc -l
556

However, the changes other than the errors.New() edits in here are OK, would you mind scaling it back to non-capital letter error string starts and the typo fixes?

@ChengenH
Copy link
Contributor Author

Thanks for the contribution. As nice as this is, it's not a hard standard that we enforce in this codebase, in fact we use xerrors more than errors, but even that's not consistent except where we're wrapping errors.感谢您的贡献。尽管这很好,但我们在这个代码库中强制执行它并不是一个硬性标准,事实上我们使用 xerrors 的次数多于错误,但即使这样也不一致,_除非_我们包装错误。

This PR just opens the door to a larger change-set that I don't think I want to encourage as small PRs because it'll just be highlighting the inconsistency rather than solving it:这个 PR 只是为更大的变更集打开了大门,我认为我不想作为小 PR 来鼓励它,因为它只会突出不一致而不是解决它:

$ git grep -E 'fmt\.Errorf\("[^%]*"\)' -- "*.go" | wc -l
556

However, the changes other than the errors.New() edits in here are OK, would you mind scaling it back to non-capital letter error string starts and the typo fixes?但是,除错误外的更改。这里的 New() 编辑是可以的,您介意将其缩小到非大写字母错误字符串开头和拼写错误修复吗?

image
I think some specific terms can start with a capital letter.

@rvagg
Copy link
Member

rvagg commented Dec 11, 2024

Yes, they can, I was just suggesting the ones you found in cli/chain.go

@ChengenH
Copy link
Contributor Author

ChengenH commented Dec 11, 2024

Yes, they can, I was just suggesting the ones you found in cli/chain.go

I understand, I will close this PR and open a new one in #12773.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ☑️ Done (Archive)
Development

Successfully merging this pull request may close these issues.

2 participants