Enable all linters and resolve all outstanding lint issues (Resolves issue 2386)
As per https://github.com/authzed/spicedb/issues/2386, this PR uncomments gocritic, govet, and testifylint, and resolves all outstanding lint findings. Note that govet did not actually return anything, but gocritic and testifylint did. Those were resolved, along with a number of stubborn yaml comment spacing complaints within the github workflows yamls.
I generally resolved each finding in its own commit, for clarity. Additionally, I ran the full suite of unit tests after each fix and ensured no bloat in test execution time. The test suite took ~2.5 minutes before and after all changes on my machine.
Before:
See this for the raw output of all findings as of main/HEAD
After:
mage lint:all ✔ at 04:25:57 PM
generating buf
generating go
Generating options for common.SchemaInformation...
Generated 1 options
Generating options for graph.ConcurrencyLimits...
Generated 1 options
Generating options for options.ExperimentalServerOptions...
Generated 1 options
Generating options for datastore.ConnPoolConfig...
Generated 1 options
Generating options for datastore.Config...
Generated 1 options
Generating options for datastore.RelIntegrityKey...
Generated 1 options
Generating options for server.CacheConfig...
Generated 1 options
Generating options for server.MiddlewareOption...
Generated 1 options
Generating options for server.Config...
Generated 1 options
Generating options for testserver.Config...
Generated 1 options
Generating options for util.GRPCServerConfig, HTTPServerConfig...
Generated 1 options
Generating options for options.QueryOptions, ReverseQueryOptions, RWTOptions...
Generated 1 options
Generating options for options.DeleteOptions...
Generated 1 options
running vulncheck
formatting go
running golangci-lint
running analyzers
0 issues.
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅
I have read the CLA Document and I hereby sign the CLA
@vroldanbet It's a wall of changes, but ready for you to look! I took great care in validating each one, ensuring it logically made sense and wouldn't change the spirit of each test or function. Feel free to suggest any changes - most are cut and dry but a few (mostly the Requires within a goroutine issues) could have been written a number of different ways.
Codecov Report
Attention: Patch coverage is 70.05988% with 50 lines in your changes missing coverage. Please review.
Project coverage is 77.35%. Comparing base (
a97e9c3) to head (b1ee733).
Additional details and impacted files
@@ Coverage Diff @@
## main #2455 +/- ##
==========================================
- Coverage 77.39% 77.35% -0.03%
==========================================
Files 414 414
Lines 50718 50733 +15
==========================================
- Hits 39246 39238 -8
- Misses 9002 9026 +24
+ Partials 2470 2469 -1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
@tstirrat15 Anything else I need to get this merged before it gets stale? Unable to merge it myself - not sure if @vroldanbet has to review or if he was auto-included.
@James9074 just wanted to update - this is still something we're interested in and we haven't had the bandwidth to look at it yet. Apologies for the delay.
@James9074 we're finally done with the push and should have some more bandwidth now. do you have the time and energy to get this rebased and ready? If not, we can take over the PR and make sure that you're attributed.
@James9074 we're finally done with the push and should have some more bandwidth now. do you have the time and energy to get this rebased and ready? If not, we can take over the PR and make sure that you're attributed.
Awesome! No worries, I'll rebase this week and resubmit for review.
@tstirrat15 Feel free to take this one over or close it out, unfortunately I'm short on time this quarter and don't think I'll be able to wrap this up before it's too stale.
Sounds good. Thank you so much for the work you put into it!
Thank you @James9074! Your hard work doesn't go unnoticed. We'll take this over the finish line :)
All of the individual pieces of this have been brought in. Thank you for putting this up!