Skip to content

Conversation

@FGasper
Copy link
Collaborator

@FGasper FGasper commented Jul 15, 2025

This includes code updates to satisfy the new (more stringent) linter rules.

@FGasper FGasper requested a review from a team as a code owner July 15, 2025 17:12
@FGasper FGasper requested review from zhenxuanjameszhang and removed request for a team July 15, 2025 17:12
docType := docValue.Type()
docKind := docType.Kind()
if docKind == reflect.Map {
switch docKind {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the linter require switch statement instead of if else block? I feel both style should be valid.

if err != nil {
var commandErr mongo.CommandError
if !(errors.As(err, &commandErr) && commandErr.Code == 26) {
if !errors.As(err, &commandErr) || commandErr.Code != 26 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the linter require always use one style of boolean logic? I feel either way should be fine but would not mind enforcing this from now on.

imp.InputOptions.UseArrayIndexFields,
), nil
}
return NewJSONInputReader(
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit since it is not related to linter change, I feel this should be in the else statement to be more readable.

Copy link
Collaborator

@zhenxuanjameszhang zhenxuanjameszhang left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for doing this work to make our code base quality better!

I have a few questions, it would be great if you can answer them.

Could you please also summarize the lint rule change in the PR DESCRIPTION? I got some idea by reviewing but think it would be nice to have it for reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants