-
Notifications
You must be signed in to change notification settings - Fork 21
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
enable dependabot and golangci-lint #101
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #101 +/- ##
==========================================
- Coverage 99.74% 99.74% -0.01%
==========================================
Files 28 28
Lines 3152 3126 -26
==========================================
- Hits 3144 3118 -26
Misses 8 8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great additions. Here are some suggestions
- "gosimple" | ||
- "govet" | ||
- "ineffassign" | ||
- "staticcheck" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have added testifylint as testify is used in the code
But it could be addressed in another PR, maybe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i wanted to do a first batch of rules that could be resolved mostly with automatic tools
errors/parameter_errors.go
Outdated
"github.com/pb33f/libopenapi/datamodel/high/base" | ||
|
||
"github.com/pb33f/libopenapi-validator/helpers" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is reason to have a space between them for me
Here are some solutions, I'm suggesting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
localmodule is working nicely
parameters/cookie_parameters.go
Outdated
"github.com/pb33f/libopenapi/datamodel/high/base" | ||
|
||
"github.com/pb33f/libopenapi-validator/errors" | ||
"github.com/pb33f/libopenapi-validator/helpers" | ||
"github.com/pb33f/libopenapi-validator/paths" | ||
|
||
v3 "github.com/pb33f/libopenapi/datamodel/high/v3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You used alias in gci, but I find the result strange here
"github.com/pb33f/libopenapi/
"github.com/pb33f/libopenapi-validator/
v3 "github.com/pb33f/libopenapi
Maybe one of the solutions I'm suggesting would provide something less strange
"github.com/pb33f/libopenapi/datamodel/high/base" | ||
"github.com/pb33f/libopenapi/datamodel/low" | ||
"github.com/pb33f/libopenapi/orderedmap" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect this package and everything under pb33f to be regrouped.
I'm raising it again as you resolved.
But I'm finr if you disagree with me 😁
Localmodule might have not been enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wdyt about distinct blocks for localmodule and other pb33f modules?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the last solution I suggested here
So of course, I'm OK with it.
But please note, I'm a random Gopher. I'm not a maintainer of this repository.
Maybe @daveshanley will have another point of view. But whatever the gci config, I'm fine with it.
I'm looking for enforcing the imports, as it's currently not consistent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no preference at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. LGTM!
"github.com/pb33f/libopenapi/datamodel/high/base" | ||
"github.com/pb33f/libopenapi/datamodel/low" | ||
"github.com/pb33f/libopenapi/orderedmap" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no preference at all.
@@ -197,7 +199,7 @@ func formatJsonSchemaValidationError(schema *base.Schema, scErrs *jsonschema.Val | |||
if schema != nil { | |||
rendered, err := schema.RenderInline() | |||
if err == nil && rendered != nil { | |||
fail.ReferenceSchema = fmt.Sprintf("%s", rendered) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea why I did this.
Dependabot to have automatic update of dependencies
Golangci-lint to ensure a coherent style