Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

MAINT: Linting #1380

Merged
merged 4 commits into from
Jan 27, 2022
Merged

MAINT: Linting #1380

merged 4 commits into from
Jan 27, 2022

Conversation

tlimoncelli
Copy link
Contributor

@tlimoncelli tlimoncelli commented Jan 25, 2022

I fixed many, many, lint and staticcheck warnings and errors. Sorry for the mega-PR but its the fastest way to get them all done.

CC @tresni @Papakai @blackshadev @mikenz @philhug @tlimoncelli

Note: I haven't used the checklist feature before. Let's see if this works. I've made a checklist of the files in this PR. We'll check them off as people confirm them. I may check off some myself if they are just comments.)

  • OWNERS
  • models/record.go
  • pkg/acme/acme.go
  • pkg/js/js.go
  • pkg/normalize/validate.go
  • providers/cloudflare/cloudflareProvider.go
  • providers/cscglobal/api.go
  • providers/cscglobal/cscglobalProvider.go
  • providers/easyname/api.go
  • providers/easyname/provider.go
  • providers/hexonet/domains.go
  • providers/opensrs/opensrsProvider.go
  • providers/route53/route53Provider.go
  • providers/transip/transipProvider.go

@tlimoncelli
Copy link
Contributor Author

@blackshadev
Copy link
Contributor

Hi @tlimoncelli , what can I do for you?

If you want me to check the changes in the provider I am an maintainer (Transip): You only added a comment and it is perfect.
If you need more: could you please explain what I can do for you?

@tlimoncelli
Copy link
Contributor Author

tlimoncelli commented Jan 25, 2022

Hi @tlimoncelli , what can I do for you?

If you want me to check the changes in the provider I am an maintainer (Transip): You only added a comment and it is perfect. If you need more: could you please explain what I can do for you?

Yes, verifying the changes related to TransIP is all I needed. (You had it easy: other people had bigger changes.)

Thanks for checking in!

(Edit: Clarified which changes)

@tresni
Copy link
Contributor

tresni commented Jan 26, 2022

Easyname looks good, just the one change for Id -> ID

@@ -21,6 +21,9 @@ func (n *HXClient) EnsureDomainExists(domain string) error {
}
} else if code == 531 {
return n.GetHXApiError("Not authorized to manage dnszone", domain, r)
// FIXME(tlim) go-staticcheck reports:
// identical expressions on the left and right side of the '||' operator (SA4000)
// Perhaps the right side should be n.IsError() or deleted?
} else if r.IsError() || r.IsError() {
Copy link
Contributor

@KaiSchwarz-cnic KaiSchwarz-cnic Jan 26, 2022

Choose a reason for hiding this comment

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

Morning @tlimoncelli

I suggest to rewrite

} else if r.IsError() || r.IsError() {

to

} else if r.IsError() || r.IsTmpError() {

Probably an oversight on my end some time ago.
Find the Reference to IsTmpError function here.

That's it basically :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed as suggested. If it passed CI, I'll mark this as done.

@tlimoncelli
Copy link
Contributor Author

Sorry to rush folks but I'd like to do a release soon, so I'm going to merge this. The changes are rather low-risk. The new release will go out early next week but first we're going to use it inside Stack Overflow for a bit as we usually do.

@tlimoncelli tlimoncelli merged commit dcb0e58 into master Jan 27, 2022
@tlimoncelli tlimoncelli deleted the tlim_lint3 branch January 27, 2022 20:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants