Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Add 'file' option for lintOnSave setting #1965

Merged
merged 6 commits into from
Oct 12, 2018

Conversation

brainsnail
Copy link
Contributor

What was added

  • Adds the feature described on Issue 1931 - enabling lint on save on an individual file basis

What was tested

I sideloaded the extension then tested the following linters in "go.lintOnSave" file mode by adding some unused variables and uncommented functions in a Go project.

  • golint
  • gometalinter
  • megacheck
  • golangci-lint
  • revive

Please let me know if there's anything I've overlooked or if anything be improved. Thanks!

@msftclas
Copy link

msftclas commented Oct 4, 2018

CLA assistant check
All CLA requirements met.

src/goLint.ts Outdated
*/
export function goLint(fileUri: vscode.Uri, goConfig: vscode.WorkspaceConfiguration, lintWorkspace?: boolean): Promise<ICheckResult[]> {
export function goLint(fileUri: vscode.Uri, goConfig: vscode.WorkspaceConfiguration, lintWorkspace?:boolean, lintFile?: boolean): Promise<ICheckResult[]> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Using 2 separate variables to manage scope feels excessive. Why not use a single variable, say scope? This can have one of the 4 values file, package, workspace, off.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I was wondering about this. I took your advice and passed a string value around. It cleans up the signatures.

I appreciate the feedback 😄

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

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

Hey @brainsnail,

I have added one comment on the code. Please do take a look.
Apart from that, the linter is failing due to some extra trailing spaces.
Format the files to get rid of that.
You can also run npm run lint in the end to make sure there are no linting errors.

Also, dont forget to register for the Microsoft Hacktoberfest :)

@ramya-rao-a ramya-rao-a merged commit c6e4925 into microsoft:master Oct 12, 2018
@ramya-rao-a
Copy link
Contributor

Thanks!

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.

3 participants