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

chore: migrate to cli-table3 #115

Merged
merged 3 commits into from
Oct 24, 2018
Merged

chore: migrate to cli-table3 #115

merged 3 commits into from
Oct 24, 2018

Conversation

DanielRuf
Copy link
Contributor

No description provided.

@brandonocasey
Copy link

It would be really good to get this in to resolve the security issue, but I think it means dropping support for node <= 4, which would be a major version bump.

@DanielRuf
Copy link
Contributor Author

Node.js 4 reached its EOL and will not get any security updates.

Do you want to support it? If yes, you can use the previous release of cli-table3.

@brandonocasey
Copy link

sorry for the confusion, I am not a maintainer here, I just wanted to make sure that everything has been discussed for this so that it can go in quickly

@mattallty
Copy link
Owner

Node.js 4 reached its EOL and will not get any security updates.

Do you want to support it? If yes, you can use the previous release of cli-table3.

You're right, we can drop support of it (so major version bump)

@DanielRuf thank you for the PR, could you please fix conflicts ?

@dannypaz
Copy link

@mattallty @DanielRuf let me know and I can make the fix w/ conflicts on another PR. We should update as cli-table2 is abandoned

@DanielRuf
Copy link
Contributor Author

DanielRuf commented Oct 15, 2018

@DanielRuf thank you for the PR, could you please fix conflicts ?

Sorry, forgot to react. I'll rebase and resolve the conflicts (tomorrow morning).

@DanielRuf
Copy link
Contributor Author

Sorry, forgot to react. I'll rebase and resolve the conflicts (tomorrow morning).

Done.

@dannypaz
Copy link

@mattallty is there anything we need to do RE: CHANGELOG or cutting a release?

@mattallty mattallty merged commit e949a4e into mattallty:master Oct 24, 2018
@DanielRuf DanielRuf deleted the chore/migrate-to-cli-table3 branch October 24, 2018 19:51
@mattallty
Copy link
Owner

Just merged into master, if you guys could play with it, I will be happy to publish it on npm if no one suffers :) @dannypaz @DanielRuf @brandonocasey

@brandonocasey
Copy link

This appears to work for es-check as expected

@dannypaz
Copy link

@mattallty we've tested the version of caporal on 2 of our applications and everything looks good.

@dannypaz
Copy link

dannypaz commented Nov 2, 2018

@mattallty let us know when the release is cut. The only issue that I came across was having some options filled out incorrectly, that caused some custom validations to fail.

// No longer works correctly with validation. The default value had changed from empty string to nil
.option('--market', 'Relevant market name', validations.isMarketName)

// This fixes the return value to the original functionality
.option('--market [marketname]', 'Relevant market name', validations.isMarketName)

Not sure if it is worth mentioning in the release notes. If you want me to dig up where this change was made in cli-table3, let me know.

@dannypaz
Copy link

@mattallty @DanielRuf Thank you both again for the help/work in getting this functionality in 🎊

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