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

Use yargs to parse the arguments passed to the linter #2155

Merged
merged 2 commits into from
Nov 8, 2018

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented May 25, 2018

This makes it possible to manually lint multiple files at once without needing to run the linter multiple times.

Also probably fixes the odd issue where an error in a single file would cause all subsequent files to be printed at the end, which is also present in the build for #2156.


Depends on #2288

P.S.: yargs@^11.1.0 is already present in the tree because of mdn‑confluence.

@Elchi3 Elchi3 added the linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files. label May 25, 2018
@ExE-Boss ExE-Boss force-pushed the linter/yargs branch 2 times, most recently from 968f561 to 7f8e998 Compare June 20, 2018 17:31
  This makes it possible to manually lint multiple files at once
without needing to run the linter multiple times.

  Also probably fixes the issue where an error in a single file would
cause all subsequent files to be printed at the end¹.

# References:
¹ https://travis-ci.org/mdn/browser-compat-data/builds/383532922#L7191
Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

I think this would be a nice addition, thanks @ExE-Boss. Like with the other PR about better error messages, the dependency now comes with a newer version, which we should use. Sorry about the delay.

@@ -26,7 +26,8 @@
"homepage": "https://github.com/mdn/browser-compat-data#readme",
"devDependencies": {
"ajv": "^5.0.1",
"mdn-confluence": "0.0.7"
"mdn-confluence": "0.0.7",
"yargs": "^11.1.0"
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for not having a look at this earlier, but meanwhile yargs comes with version 12.

Copy link
Contributor Author

@ExE-Boss ExE-Boss Nov 5, 2018

Choose a reason for hiding this comment

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

Well, yargs was at version 11 when I made this, and it’s version 11.1 that’s in the node_modules tree:

"yargs": {
"version": "11.1.0",

Copy link
Member

Choose a reason for hiding this comment

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

Alright, let go with this version for now then.

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

Thank you, works great! 👍

@Elchi3 Elchi3 merged commit 8916bf9 into mdn:master Nov 8, 2018
@ExE-Boss ExE-Boss deleted the linter/yargs branch November 8, 2018 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants