Skip to content
This repository has been archived by the owner on Oct 16, 2018. It is now read-only.

Include metalinter config #26

Merged
merged 2 commits into from
Apr 3, 2018
Merged

Include metalinter config #26

merged 2 commits into from
Apr 3, 2018

Conversation

prateek
Copy link
Collaborator

@prateek prateek commented Apr 3, 2018

No description provided.

@prateek prateek force-pushed the prateek/fix-metalint-issues branch from 266e6a9 to 4ea1281 Compare April 3, 2018 19:54
@prateek prateek changed the title [WIP] Include metalinter config Include metalinter config Apr 3, 2018
@prateek prateek requested a review from jeromefroe April 3, 2018 19:54
Copy link
Contributor

@jeromefroe jeromefroe left a comment

Choose a reason for hiding this comment

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

LGTM

"golint",
"gosimple",
"unused",
"maligned"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth adding go vet and go fmt as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

opened #27 to track


err error
closed bool
maxID postings.ID
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this because of structcheck? If so, is it worth preferring the old layout since it had the benefit of distinguishing immutable versus mutable fields.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's because of maligned: this layout is 80 bytes, the older one was 88.

I'll put a comment to indicate why we order this way and the mutability of fields.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@prateek prateek merged commit 35a0d09 into master Apr 3, 2018
@prateek prateek deleted the prateek/fix-metalint-issues branch April 3, 2018 20:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants