-
Notifications
You must be signed in to change notification settings - Fork 5
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 table to display mismatching version output #197
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.
I think a table clutters the output too much and may add too many additional lines to the output when more features are added. The output should match what is seen with similar tools like linters and testing frameworks.
The data we want to display is tabular. Tables make it easy to parse the information, and will allow us to conveniently add more information over time. In particular, I'd like to add another column containing the list of packages for each version (#187). Many popular tools use tables to display output: nyc code coverage from
The exact formatting/style of the table is something that we can tweak later if desired, but the key win here is getting the data into tabular format to start with. |
@bmish The tables in your examples have different formatting than what you have in this PR. Right now there’s one table per dependency rather than one large table for the entire report like in your example. Having one large table would reduce the number of lines of output. Additionally, the example tables span the entire width of the terminal window, whereas this implementation has variable-width tables, which seem to be a poor design choice. And the tables in the screenshots do not have lines separating each row in the table body, which reduces the total number of lines of output. |
Thanks for your concerns. I'll be happy to consider updates to the formatting later, but the table is needed to unblock upcoming changes. It's already a big improvement to readability and extensibility. |
`${chalk.bold(obj.dependency)} has more than one version:\n${obj.versions | ||
): string { | ||
if (mismatchingDependencyVersions.length === 0) { | ||
throw new Error('No mismatching versions to 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.
Should an error be thrown when there are no mismatches?
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.
Yes because mismatchingVersionsToOutput
should only be called when there are mismatching versions (that's how the code is written now). When there are no mismatches, there is no output.
Fixes #188.
Screenshot