-
Notifications
You must be signed in to change notification settings - Fork 29
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
Pull in pull in summary line generation #108
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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!
@@ -0,0 +1,49 @@ | |||
'use strict'; | |||
|
|||
const chalk = require('chalk'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's include a comment here that this is taken from eslint-summary, with a link to the github and a copy of its license. Credit where it's due, and all. :)
@@ -72,21 +73,21 @@ test('cli :: returns exit code without menu when --no-interactive is set', async | |||
t.equal(await cli.execute([nodeBin, nibbleBin, okayPath, '--no-interactive']), 0, 'exits with 0 if no problems'); | |||
console.log = function (input) { | |||
console.log = origConsoleLog; | |||
t.ok(input.includes('No lint failures found for rule(s): prefer-const', 'Warns user')); | |||
t.ok(stripAnsi(input).includes('No lint failures found for rule(s): prefer-const', 'Warns user')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any idea why this started to fail after the changes here? I'm a bit surprised that they would have passed before and started failing now, since I think you just copied over the code from the dependency. Maybe it's now using an updated version of chalk or text-table, which is changing the output?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, I checked out master, and they appear to have been failing on master as well. I'll double check myself on that. 👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you on Windows or something, by chance? I checked out master and it passed fine, and I removed these from your branch and it works fine too. But maybe your environment is different.
Fixes #107
Background
eslint-summary
provides the summary line foreslint-nibble
.While
eslint-nibble
has been maintained,eslint-summary
has not for ~8yrs. For exampleeslint-nibble
is using"chalk": "^4.1.1"
whileeslint-summary
is still on "chalk": "^1.0.0".The functionality of
eslint-summary
is rather straightforward. Does it make sense to adopt and pull this code intoeslint-nibble
?This came to my attention when looking through my repos dependencies and vulnerabilities and chasing down where older versions of
chalk
where coming from.See also
Issue #107 Discussion