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(lint): use Go stdlib variables for HTTP methods and status codes #17968

Merged
merged 3 commits into from
Jul 26, 2023

Conversation

thevilledev
Copy link
Contributor

@thevilledev thevilledev commented Jul 18, 2023

What

  • Modify net/http related calls to consistently use HTTP methods and response codes from variables/constants provided by the pkg.
  • Add usestdlibvars to lint configuration.

See #18061 and #18062 for changes related to node pool and variable endpoints.

Why

Some parts of the codebase already use these. With this PR, better code consistency and reliability.

Copy link
Member

@jrasell jrasell left a comment

Choose a reason for hiding this comment

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

Hi @thevilledev and thanks so much for this PR, it LGTM.

Before merging I wanted to ask whether it would be possible to split this up slightly into 3 separate PR's, which would help backporting. Ideally the PR's would address the following areas, so I can backport to different release branches:

  • node pools
  • variables
  • everything else

Being particular about backporting will make future development and backporting easier.

@thevilledev
Copy link
Contributor Author

Happy to help and thank you for the review @jrasell!

In terms of backporting, I can definitely split this into three separate PRs. Just to confirm, would this splitting work?

  • PR1: Node pools (node_pool_endpoint.go and node_pool_endpoint_test.go)
  • PR2: Variables (variable_endpoint.go and variable_endpoint_test.go)
  • PR3: Everything else

@jrasell
Copy link
Member

jrasell commented Jul 25, 2023

Happy to help and thank you for the review @jrasell!

In terms of backporting, I can definitely split this into three separate PRs. Just to confirm, would this splitting work?

  • PR1: Node pools (node_pool_endpoint.go and node_pool_endpoint_test.go)
  • PR2: Variables (variable_endpoint.go and variable_endpoint_test.go)
  • PR3: Everything else

Exactly; thanks again @thevilledev!

@thevilledev
Copy link
Contributor Author

Alright, all done! I removed those two changes from this PR so please consider this as the third PR consisting of changes for the rest of the files and linting 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line backport/1.6.x backport to 1.6.x release line
Projects
Development

Successfully merging this pull request may close these issues.

2 participants