[LINT] Remove fix option for ESLint

[harupy]

What changes are proposed in this pull request?

This PR propses to remove --fix option from server/js/lint.sh (link) because with --fix option, eslint exits with 0 if all the detected violations are fixable. This behavior might allow code that violates the lint rules to pass the CI tests.

How is this patch tested?

Tested on this PR #2035

Release Notes

Is this a user-facing change?

• No. You can skip the rest of this section.
• Yes. Give a description of this change to be included in the release notes for MLflow users.

(Details in 1-2 sentences. You can just refer to another PR with a description if this PR is part of a larger change.)

What component(s) does this PR affect?

• UI
• CLI
• API
• REST-API
• Examples
• Docs
• Tracking
• Projects
• Artifacts
• Models
• Scoring
• Serving
• R
• Java
• Python

How should the PR be classified in the release notes? Choose one:

• rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
• rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
• rn/feature - A new user-facing feature worth mentioning in the release notes
• rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
• rn/documentation - A user-facing documentation change worth mentioning in the release notes

[Zangr]

Good point on removing this to prevent false positive in CI test. @smurching, maybe we can have a separate script for CI(lint_ci.sh)? I personally find --fix pretty useful during development.

[aarondav]

Agree with @Zangr. I also use this script during development, and it's nice that it does in place fixing of certain style issues. Let's have a separate variant for CI which does not fix.

[harupy]

@Zangr @aarondav Thanks for the comments. I also agree with you two.

[harupy]

I personally prefer to include lint commands into npm scripts like:

"scripts": {
  ...
  "lint": "eslint src",
  "lint:fix": "eslint src --fix",
  ...
}

[Zangr]

@harupy yes that makes sense.

[harupy]

@aarondav What do you think?

[aarondav]

Sorry, I am not as familiar with npm -- would this mean developers would run npm:lint and npm lint:fix? That seems reasonable.

[harupy]

You would run:

npm run lint
npm run lint:fix

[aarondav]

Sounds good, let's do it

[Zangr]

@aarondav , yes we need to change the CI script accordingly.

[harupy]

Should I remove lint.sh?

[aarondav]

Yes, sounds good.

[aarondav]

Tried it out locally and it seems to work as expected, thanks for the fix!