-
Notifications
You must be signed in to change notification settings - Fork 408
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: Fix lint warnings from golangci-lint #3311
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes detected by Check Git Status Action
I concur with the proposed
|
The |
I concur with the proposed |
@@ -133,7 +132,7 @@ func initAxfrDdns(config map[string]string, providermeta json.RawMessage) (provi | |||
} else if len(api.nameservers) != 0 { | |||
api.master = api.nameservers[0].Name + ":53" | |||
} else { | |||
return nil, fmt.Errorf("nameservers list is empty: creds.json needs a default `nameservers` or an explicit `master`") | |||
return nil, errors.New("nameservers list is empty: creds.json needs a default `nameservers` or an explicit `master`") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this line changed to errors.New
while lines 206, 223, 227, 287, and 387 still use fmt.Errorf
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question!
errors.New() is appropriate when there is no formatting needed.
- fmt.Errorf(): Has "printf" semantics
- errors.New(): has "print" semantics
[x] providers/porkbun/ @imlonghao LGTM By the way, can we make those lints in GitHub Actions and executed when doing PR? |
Yes! Noted here: #3313 (I could use a volunteer) |
I'm not sure if I've offended the gods of namecheap or there is a genuine problem here but I'm getting a timeout during one of the tests. I gave it half an hour and tried again and it was still happening so not sure if I should try again tomorrow or not. Stack traces under the cut stack traces
|
LGTM |
LGTM |
@willpower232 Did the problem go away on its own? |
Looks good, thanks. |
@tlimoncelli yeah it seems to, some kind of extra rate limiting is in place I'd say, I'm able to make it work successively if I wait an hour haha the same five tests fail on main and this branch so looks like the namecheap provider is just as good in both places 🙏
|
Agreed. That said... could you open a bug about those 5 items? Tom |
LGTM |
Friendly reminder! Merge happens on Monday. |
Issue
golangci-lint reports many, many, many warnings
Resolution
Fix them all!
Please check my changes!
Please check your code and give feedback or mark it approved:
I'll be merging this on Jan 13, 2025. Please get your feedback in by then.
Notes
Here are (some of) the warnings: