search

ryanrolds / sqlclosecheck

Linter that confirms DB rows and statements are closed properly.
MIT License
83 stars 15 forks source link

Feature/closure param support #37

Open khchehab opened 4 months ago

khchehab commented 4 months ago

Hi, this is my first contribution and I hope I did this right, please let me know if I missed anything.

When closing a connection/statement/row set I use defer blocks with parameters of what I'm closing. This was not being checked and using golangci-lint with this linter reported these errors and that the row set was not being closed.

I added this check so that it takes into consideration if a close was done inside a defer block closure that takes in a parameter as well.

I added test cases for successes and failures as well.

I also updated the packages since there was an issue when building it without any changes, due to a panic from one of the indirect dependencies.

khchehab commented 4 months ago

Hi @ccoVeille, thank you for your feedback. I checked your review points and and I agree with them but I did not change anything that was existing since I don't know if the author of the repo intended to use certain approaches or not. I only added the feature that I needed to support.

Do you think we should change it?

ccoVeille commented 4 months ago

I agree with you. As I said I didn't look at the code base.

ccoVeille commented 4 months ago

Let's wait for @ryanrolds to look at them. But I'm fine with your changes @khchehab, no need to address my comments before merging

This one is the one that maybe addressed in another issue if Ryan finds it useful

https://github.com/ryanrolds/sqlclosecheck/pull/37#discussion_r1656557448

ryanrolds commented 3 months ago

Thank you for your patience, I've been busy isn't the case anymore. I will work reviewing this into my todos this week.

Thank you.