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

Add ESLint Task #774

Merged
merged 7 commits into from
Jun 24, 2020
Merged

Add ESLint Task #774

merged 7 commits into from
Jun 24, 2020

Conversation

MitchellMcKenna
Copy link
Contributor

@MitchellMcKenna MitchellMcKenna commented Jun 12, 2020

Q A
Branch master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Documented? yes
Fixed tickets no

Add ESLint task for javascript code static analysis (code quality and coding style).

As a good amount of php projects also have some javascript frontend, it's handy to lint your JS during pre-commit as well.

Example task run:

$ git commit
GrumPHP detected a pre-commit command.
GrumPHP is sniffing your code!

Running task 1/4: phpcs... 
Running task 2/4: phpunit... 
Running task 3/4: phpstan... 
Running task 4/4: eslint... ✘

eslint
======

/resources/js/app.js
  23:1   error    Expected indentation of 4 spaces but found 8  indent

✖ 1 problem (1 error, 0 warnings)
  1 error potentially fixable with the `--fix` option.

You can fix all errors by running following commands:
'eslint' 'resources/js/app.js'  '--fix'
To skip commit checks, add -n or --no-verify flag to commit command

 I can fix some stuff automatically, do you want me to? (yes/no) [no]:
 > 

New Task Checklist:

  • Are the dependencies added to the composer.json suggestions? (n/a)
  • Is the doc/tasks.md file updated?
  • Are the task parameters documented?
  • Is the task registered in the tasks.yml file?
  • Does the task contains phpunit tests?
  • Is the configuration having logical allowed types?
  • Does the task run in the correct context?
  • Is the run() method readable?
  • Is the run() method using the configuration correctly?
  • Are all CI services returning green?

- Add ESLint task for javascript code static analysis..
- Add ESLint test.
- Add eslint.md instructions.
- Update tasks.md to add link to instructions.
Copy link
Contributor

@veewee veewee left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! It looks pretty good. I've added some small remarks.

Copy link
Contributor Author

@MitchellMcKenna MitchellMcKenna left a comment

Choose a reason for hiding this comment

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

Thanks for feedback. I will add fixer and update unit test.

Wow, you reviewed this like right away, impressive as I submitted it at like 2am my time, lol.

@MitchellMcKenna MitchellMcKenna requested a review from veewee June 20, 2020 18:50
- Simplify the formatErrorMessage() since it really only needs
  2 strings, not an array.
- Update the ESLintFormatterSpec to test result, instead of just
  that it gets a string back.
@veewee veewee added this to the 0.19.1 milestone Jun 24, 2020
@veewee veewee merged commit 82b7feb into phpro:master Jun 24, 2020
@veewee
Copy link
Contributor

veewee commented Jun 24, 2020

Thanks for your work. Really nice!

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

Successfully merging this pull request may close these issues.

2 participants