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

deps(chore): switch to cli-table3 to resolve security issue #302

Closed
wants to merge 1 commit into from
Closed

deps(chore): switch to cli-table3 to resolve security issue #302

wants to merge 1 commit into from

Conversation

schalkneethling
Copy link

This PR updates the cli-table2 dependency to cli-table3, which fixes one of the npm audit warnings :)

cli-table2 (like cli-table itself) is no longer maintained. In jamestalmage/cli-table2#43 a couple of people have offered to take over maintenance but the current maintainer did not respond so as a result the project was forked to cli-table/cli-table3.

@coveralls
Copy link

coveralls commented Jun 11, 2018

Coverage Status

Coverage decreased (-14.6%) to 85.35% when pulling 91cbe4f on schalkneethling:issue#297-resolve-cli-table-dependency into c191adf on zemirco:master.

@knownasilya
Copy link
Collaborator

Looks like node 4 and 5 don't work due to Block-scoped declarations (let, const, function, class) not yet supported outside strict mode.

Also color shows up in Node 8 and 9 but not in 6 and 7. We are testing against the output without the colors, so tests fail in 8 and 9.

@schalkneethling
Copy link
Author

Thanks for reporting that @knownasilya - I will open an issue for that over on the new repo

@schalkneethling
Copy link
Author

schalkneethling commented Jun 11, 2018

@knownasilya Had a quick chat with the maintainers of cli-table3 and we are setting the lowest Node.js to 6+ - https://github.com/cli-table/cli-table3/blob/v0.5.0/package.json#L60

cli-table/cli-table3#19 (comment)

@knownasilya
Copy link
Collaborator

Ok, that's fine for v4 and v5 not being supported. What about colors coming through in 8 and 9 only while not in 6 and 7?

@Turbo87
Copy link

Turbo87 commented Jun 11, 2018

We are testing against the output without the colors, so tests fail in 8 and 9.

how did you disable colors?

@knownasilya
Copy link
Collaborator

@Turbo87 we aren't, it just works differently across the two sets of Node versions.

@Turbo87
Copy link

Turbo87 commented Jun 11, 2018

we actually had similar issues internally for the cli-table3 test suite, and they seemed to have been related to the color support detection. we are currently running our test suite explicity with --color to make sure that colors are always on.

@knownasilya
Copy link
Collaborator

knownasilya commented Jun 11, 2018

Can we have a --no-color option? That would make it much more readable for test results. 😸

@Turbo87
Copy link

Turbo87 commented Jun 11, 2018

--no-color should already be supported, but it's generally recommended to use the style: { head: [], border: [] } options in testing mode anyway

@knownasilya
Copy link
Collaborator

knownasilya commented Jul 12, 2018

Closed in favor of #312

Thanks for the contribution and getting this moving.

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

Successfully merging this pull request may close these issues.

4 participants