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

feat(outdated): add long option, no details by default #2017

Merged
merged 3 commits into from
Sep 18, 2019
Merged

feat(outdated): add long option, no details by default #2017

merged 3 commits into from
Sep 18, 2019

Conversation

aparajita
Copy link
Contributor

Previously, by default the outdated commands displayed a “Details” column which usually contained the URL to the package repo. As the URL could be quite long, on narrow terminals it would cause the table to wrap in a way which ruined the table borders.

This commit hides the “Details” column by default, and displays the column if --long is passed to the command.

Previously, by default the `outdated` commands displayed a “Details” column which usually contained the URL to the package repo. As the URL could be quite long, on narrow terminals it would cause the table to wrap in a way which ruined the table borders.

This commit hides the “Details” column by default, and displays the column if `--long` is passed to the command.
@aparajita aparajita requested a review from zkochan as a code owner September 18, 2019 17:20
@aparajita
Copy link
Contributor Author

@zkochan I updated the tests for outdated, but now I realize that all of the other tests will fail because details are not shown by default. There are three possible ways to solve this:

  • Show details by default, hide them with some other option.
  • Add '--long' to all of the test exec statements.
  • Modify the output in all of the tests (which I really don't want to do).

@zkochan
Copy link
Member

zkochan commented Sep 18, 2019

@zkochan I updated the tests for outdated, but now I realize that all of the other tests will fail because details are not shown by default.
...

  • Modify the output in all of the tests (which I really don't want to do).

But you did that. I see that you updated the outputs and added new test cases for outdated --long. And only one test fails that you should look into

Copy link
Member

@zkochan zkochan left a comment

Choose a reason for hiding this comment

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

Please, document this new flag of outdated in cmd/help.ts

@aparajita
Copy link
Contributor Author

But you did that.

I wasn't thinking clearly... I was thinking that the change in the outdated output would affect other commands, but of course it won't. 🙄

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@zkochan zkochan merged commit 85faad5 into pnpm:master Sep 18, 2019
@zkochan zkochan added this to the v4.0 milestone Sep 18, 2019
@aparajita aparajita deleted the no-details branch September 18, 2019 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants