Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: show eslint warnings #368

Merged
merged 2 commits into from
Mar 15, 2021
Merged

feat: show eslint warnings #368

merged 2 commits into from
Mar 15, 2021

Conversation

varl
Copy link
Contributor

@varl varl commented Mar 12, 2021

This removes the --quiet flag from the ESLint invocation, and fixes
the broken output where ESLint complains about us trying to lint
dot-files and folders by un-ignoring their built-in ignore filters.

We agree with ignoring node_modules in terms of linting however, so that
is left in place.

Supersedes: #367
Supersedes: #366

@varl varl changed the base branch from master to alpha March 12, 2021 08:27
This removes the `--quiet` flag from the ESLint invocation, and fixes
the broken output where ESLint complains about us trying to lint
dot-files and folders by un-ignoring their built-in ignore filters.

We agree with ignoring node_modules in terms of linting however, so that
is left in place.
@varl
Copy link
Contributor Author

varl commented Mar 15, 2021

Merging to alpha, need to test this a bit in places.

@varl varl merged commit abe4668 into alpha Mar 15, 2021
@varl varl deleted the allow-warnings branch March 15, 2021 06:33
dhis2-bot added a commit that referenced this pull request Mar 15, 2021
# [8.0.0-alpha.4](v8.0.0-alpha.3...v8.0.0-alpha.4) (2021-03-15)

### Features

* commit check can read from a custom file ([366be0c](366be0c))
* show eslint warnings ([#368](#368)) ([abe4668](abe4668))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 8.0.0-alpha.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ismay
Copy link
Member

ismay commented Mar 15, 2021

Installing this version of cli-style I'm encountering something that I had with previous versions of cli-style as well:

Screenshot 2021-03-15 at 10 59 58

Yarn why shows this:

Screenshot 2021-03-15 at 11 06 25

Version in package.json is 8.0.0-alpha.4:

Screenshot 2021-03-15 at 11 08 04

It seems yarn is not hoising eslint-plugin-prettier to the top level of node_modules, but instead installing it under @dhis2/cli-style/node_modules. Which seems all right, but apparently we're not able to resolve it. Maybe the resolver for eslint is starting from the root of the repo that has installed cli-style, and that's why it can't find this dependency?

@ismay
Copy link
Member

ismay commented Mar 15, 2021

Installing eslint-plugin-prettier to the top-level node_modules seems to resolve the above. Probably worth creating a separate issue for, seems we encountered this issue last week as well: https://dhis2.slack.com/archives/CJF3MDK8Q/p1615472076142100.

So I've tested the alpha. Without changing anything to the rules this is what I'm getting:

Screenshot 2021-03-15 at 11 21 08

Two things stood out to me:

  • It seems it's not just dotfiles that are causing issues with the ignored files (see the warnings from the coverage report and the d2 config)
  • The warnings are a bit hard to distinguish from the errors. Maybe grouping them with a heading, coloring, or something of that nature would help the readability? (I guess this is unrelated, but the absolute path in the errors/warnings for me also contributes a bit to it being a bit harder to read)

Pushed an example branch here: dhis2/scheduler-app#144

@varl
Copy link
Contributor Author

varl commented Mar 15, 2021

  • It seems it's not just dotfiles that are causing issues with the ignored files (see the warnings from the coverage report and the d2 config)

Ugh. Yeah.

  • The warnings are a bit hard to distinguish from the errors. Maybe grouping them with a heading, coloring, or something of that nature would help the readability? (I guess this is unrelated, but the absolute path in the errors/warnings for me also contributes a bit to it being a bit harder to read)

Yeah.

Everything has been tuned to only consider errors, so that there are weird cases here makes sense, but not ready for prime time. :)

Thanks for trying it out!

@ismay
Copy link
Member

ismay commented Mar 15, 2021

  • It seems it's not just dotfiles that are causing issues with the ignored files (see the warnings from the coverage report and the d2 config)

Ugh. Yeah.

  • The warnings are a bit hard to distinguish from the errors. Maybe grouping them with a heading, coloring, or something of that nature would help the readability? (I guess this is unrelated, but the absolute path in the errors/warnings for me also contributes a bit to it being a bit harder to read)

Yeah.

Everything has been tuned to only consider errors, so that there are weird cases here makes sense, but not ready for prime time. :)

Thanks for trying it out!

Sure thing 👍! Let me know if I can do anything to help!

@ismay
Copy link
Member

ismay commented Mar 15, 2021

I'll create a Jira issue for the eslint dependency issue.

edit: https://jira.dhis2.org/browse/CLI-35

varl added a commit that referenced this pull request Mar 26, 2021
This removes the `--quiet` flag from the ESLint invocation, and fixes
the broken output where ESLint complains about us trying to lint
dot-files and folders by un-ignoring their built-in ignore filters.

We agree with ignoring node_modules in terms of linting however, so that
is left in place.
dhis2-bot added a commit that referenced this pull request May 5, 2021
# [8.0.0](v7.3.0...v8.0.0) (2021-05-05)

### Bug Fixes

* allow eslint to print colors ([#384](#384)) ([c95a184](c95a184))
* ignore .d2 directory ([#386](#386)) ([6a473f2](6a473f2))
* match files from project root ([b77ef35](b77ef35))
* wrong hooks where installed by default ([#385](#385)) ([3ac82f4](3ac82f4))

### Code Refactoring

* remove husky ([752b944](752b944))

### Features

* commit check can read from a custom file ([366be0c](366be0c))
* delete deprecated configuration files ([c961647](c961647))
* lint file system for consistent names ([#379](#379)) ([2bfb5ef](2bfb5ef))
* new and improved d2-style ([#378](#378)) ([f8279e5](f8279e5))
* opt-in git hooks ([#333](#333)) ([a3bc415](a3bc415))
* show eslint warnings ([#368](#368)) ([abe4668](abe4668))

### BREAKING CHANGES

* The verb (check/apply) is now moved to the top-level.  E.g.
"d2-style js check" becomes "d2-style check js". This is to allow all checkers
to run with a single command: "d2-style check"
* "d2-style install" is no longer used to set up linters.  As of
husky@5 the tool is vastly simplified and much faster. Hooks can be installed
manually with "d2-style install", but is also run as a "post-install" script
that we control for consistency. Configuration is added to the project with the
"d2-style add" command.
* Husky has been removed from cli-style. You will need to
remove hooks that reference husky.sh in .git/hooks.
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 8.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants