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

Add execinquery linter #2677

Merged
merged 12 commits into from Mar 29, 2022
Merged

Add execinquery linter #2677

merged 12 commits into from Mar 29, 2022

Conversation

lufeee
Copy link
Contributor

@lufeee lufeee commented Mar 22, 2022

execinquery is a linter that detects query at database/sql package Query function.
in detail, please check following link.
https://github.com/lufeee/execinquery

@boring-cyborg
Copy link

@boring-cyborg boring-cyborg bot commented Mar 22, 2022

Hey, thank you for opening your first Pull Request !

@ldez ldez added linter: new blocked labels Mar 22, 2022
pkg/lint/lintersdb/manager.go Outdated Show resolved Hide resolved
test/testdata/querycheck.go Outdated Show resolved Hide resolved
@sivchari
Copy link
Member

@sivchari sivchari commented Mar 22, 2022

Thanks for contributing. I commented nits.

@CLAassistant
Copy link

@CLAassistant CLAassistant commented Mar 23, 2022

CLA assistant check
All committers have signed the CLA.

@ldez ldez removed the blocked label Mar 27, 2022
@ldez
Copy link
Member

@ldez ldez commented Mar 27, 2022

Could you rebase your PR? Thanks.

@ldez ldez added the feedback required label Mar 27, 2022
@lufeee
Copy link
Contributor Author

@lufeee lufeee commented Mar 27, 2022

Sorry, I don't get the meaning.
rebase means squash the all commit, isn't it?

@ldez
Copy link
Member

@ldez ldez commented Mar 27, 2022

Sorry, I don't get the meaning.

can you rebase your branch (related to your Pull Request) on the branch master of golangci-lint?

rebase means squash the all commit, isn't it?

nope, rebase means just rebase

example:

git rebase upstream/master

@lufeee
Copy link
Contributor Author

@lufeee lufeee commented Mar 27, 2022

Thanks, i got it.

@ldez
Copy link
Member

@ldez ldez commented Mar 29, 2022

I will rebase your branch because I feel that you need help. (rebase != merge)

@ldez
Copy link
Member

@ldez ldez commented Mar 29, 2022

Your tag (v1.0) is not valid from the point of view of Go, can you create a valid tag (v1.0.0)?

can you also fix the name of your analyzer?
https://github.com/lufeee/execinquery/blob/8f5c1333d66ea66e994e3b646a4f09bb4d3b9295/execinquery.go#L16

@ldez ldez changed the title Add querycheck linter Add execinquery linter Mar 29, 2022
@lufeee
Copy link
Contributor Author

@lufeee lufeee commented Mar 29, 2022

Thanks a lot!

@lufeee
Copy link
Contributor Author

@lufeee lufeee commented Mar 29, 2022

I added new 'v1.0.0' tag and Fix analyzer name

@sivchari
Copy link
Member

@sivchari sivchari commented Mar 29, 2022

Can you update go modules ? Because this PR has used incorrect tag version's linter, yet.

go.mod Show resolved Hide resolved
@ldez
Copy link
Member

@ldez ldez commented Mar 29, 2022

For your information:

  • to update the go.sum: go mod tidy
  • a direct dependency must not be flagged as indirect.

@ldez ldez removed the feedback required label Mar 29, 2022
@lufeee
Copy link
Contributor Author

@lufeee lufeee commented Mar 29, 2022

For your information:

  • to update the go.sum: go mod tidy
  • a direct dependency must not be flagged as indirect.

Thanks for sharing your best solution about dependence.

Copy link
Member

@sivchari sivchari left a comment

LGTM

Copy link
Member

@sivchari sivchari left a comment

Sorry, one nits.

pkg/golinters/execinquery.go Outdated Show resolved Hide resolved
@sivchari
Copy link
Member

@sivchari sivchari commented Mar 29, 2022

Thanks @ldez .
LGTM

@ldez ldez merged commit b8c061e into golangci:master Mar 29, 2022
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter: new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants