sauce-docs icon indicating copy to clipboard operation
sauce-docs copied to clipboard

Add linter

Open discombobulateme opened this issue 2 years ago • 5 comments

In this PR we added: ESLint, Prettier and Husky to add pre-commit error messages. That intends to create a more conforming Docs.

discombobulateme avatar Nov 29 '22 17:11 discombobulateme

@spider-sauce @discombobulateme ~ I currently use a spell checker and the Prettier extension for Visual Studio Code, will that cause any conflicts or issues? I'd also like to better understand what the main features are for each linter (what do they check for), what a pre-commit error message will look like, and how to resolve a pre-commit error message.

lysannep avatar Nov 30 '22 16:11 lysannep

@lysannep we added the --fix flag for eslint so aside from any manual fixes that would break the build most formatting issues are enforced through the config. It shouldn't affect your extensions because it will only enforce those standards after you commit. If you're interested in which rules we enforce, you can look at the .eslintrc , and also the .prettierrc file. Let us know if you do encounter any overwritten rules or issues and we can adjust from there.

Tacktician avatar Nov 30 '22 17:11 Tacktician

@wswebcreation

TL;DR I think all of these suggestions are good and valid, it may take some time to cross train before we implement them.

I think scanning markdown files is a good approach from a content predictability standpoint, I just worry about bottlenecking the tech content team (which I'm no longer a member/core maintainer). I'll have to set up some time to cross train, create examples based on which IDE they're all using, common troubleshooting tips, etc. before we implement this.

I also want them to weigh in on which rules to enforce using prettier because they are creating the majority of the commits and I want to make sure we conform to their style guides too.

As for prettier vs. eslint, can't we use both? I doubt the tech content team will touch the JS files that much, but if they need any frontend work it would be good to enforce these rules either way. Also, I agree we should remove the --fix flag, and we can include that troubleshooting process in the cross training.

Tacktician avatar Dec 07 '22 19:12 Tacktician

Hey all,

I did some quick testing with prettier and here are the results:

  • basic markdown formatting: https://github.com/saucelabs/sauce-docs/pull/1584/files#diff-5190347e972083d2fd4bfda093482c8787f7baedbe41c95d5d93525a3848ed96
  • markdown formatting with JSX tabs: https://github.com/saucelabs/sauce-docs/pull/1584/files#diff-29c1825f272bcaddb0e62b31857b5f8237b2b194a2bec75c766337fc8f4ea0b5

I've yet to add the command to .husky, @wswebcreation will npx prettier . --check prevent the commit if it throws warn? I just want to make sure if we opt out of using --write flag that it will force folks to manually fix the formatting errors.

Tacktician avatar Dec 09 '22 19:12 Tacktician

Hi @spider-sauce

As for prettier vs. eslint, can't we use both? I doubt the tech content team will touch the JS files that much, but if they need any frontend work it would be good to enforce these rules either way.

Yes we can, prettier is more for formatting, eslint is more for code quality checks, because we have more markdown files I thought only prettier was enough, but after doing some research is better to keep them both

...will npx prettier . --check prevent the commit if it throws warn? Based on the docs it should return exit code 1, which would stop the push

wswebcreation avatar Dec 12 '22 00:12 wswebcreation