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

fix formatting of summary in cli workflow #560

Closed
ianstormtaylor opened this issue Apr 2, 2021 · 3 comments · Fixed by #698
Closed

fix formatting of summary in cli workflow #560

ianstormtaylor opened this issue Apr 2, 2021 · 3 comments · Fixed by #698

Comments

@ianstormtaylor
Copy link
Contributor

Affected Packages

pkg:cli

Problem

The summarization of a changeset in the CLI is a nice idea, but right now the formatting seems broken:

image

Proposed solution

  1. I think if you open the editor to write the changeset (instead of doing it directly in the terminal) the summary step should be eliminated. Because you've already just seen the entire changeset file and have saved and closed it.

  2. The formatting should be fixed and simplified. Potentially something like this:

🦋  === Summary of changeset ===
🦋  minor: slate, slate-react
🦋  patch: slate-history
🦋  
🦋  Note: all dependents of these packages that are incompatible with the new version 
🦋  will be patch bumped when this changeset is applied.
🦋  
🦋  Is this your desired changeset? (Y/n) · true

Which includes things like:

  • Reduce noise around the "note", using just simply red+bold on the word "Note:" will get the job done, and right now it's so noisey that it makes other pieces hard to read.
  • Reduce noise on major/minor/patch. Again color will get the job done. (And the formatting I've used mimics how they are formatted in the questions already, which is good.)
  • Collapse packages onto a single line since the majority case is have either all in one type (and a long list), or just a few packages changing, so no need to make it verbose.
  • Collapse header into a single line, since it was unnecessary to have two before.
@Andarist
Copy link
Member

Andarist commented Apr 4, 2021

I think if you open the editor to write the changeset (instead of doing it directly in the terminal) the summary step should be eliminated. Because you've already just seen the entire changeset file and have saved and closed it.

#562

Reduce noise around the "note", using just simply red+bold on the word "Note:" will get the job done, and right now it's so noisey that it makes other pieces hard to read.

I've looked into why this box was wrongly formatted - we are using boxen@^1 which doesn't handle text-wrapping. This has been implemented though so I've wanted to bump this dependency first, but I've been stopped by sindresorhus/boxen#52 . Even if that would get resolved there is still no way (from what I can tell) to "offset" the box by the length of our prefix: sindresorhus/boxen#53 . So either way - I think we should drop this dependency. I also agree with you that this is somewhat noisy.

Reduce noise on major/minor/patch. Again color will get the job done. (And the formatting I've used mimics how they are formatted in the questions already, which is good.)

As to colors - do you just propose to color major/minor/patch "labels"?

Not sure if I understand the latter part - I mean, not sure exactly how the proposed formatting mimics how packages are formatted in questions. A visual would be helpful for this.

Collapse packages onto a single line since the majority case is have either all in one type (and a long list), or just a few packages changing, so no need to make it verbose.

Agreed, not completely sure how we should treat major CLI output revamps in relation to semver. I'm inclined to say that we can be liberal about this though.

Collapse header into a single line, since it was unnecessary to have two before.

+1 on merging "Summary" and "Realising..." headers

@emmatown
Copy link
Member

emmatown commented Apr 8, 2021

This all sounds good to me.

@ianstormtaylor
Copy link
Contributor Author

As to colors - do you just propose to color major/minor/patch "labels"?

Not sure if I understand the latter part - I mean, not sure exactly how the proposed formatting mimics how packages are formatted in questions. A visual would be helpful for this.

@Andarist I just meant that currently the two places that display the "minor" concept in that screenshot are using two different types of formatting. (1) is using lowercase, color, bold. (2) is using uppercase, square brackets, non-bold, color. My suggestion is to unify on (1).

zthxxx added a commit to zthxxx/changesets that referenced this issue Dec 7, 2021
zthxxx added a commit to zthxxx/changesets that referenced this issue Dec 11, 2021
zthxxx added a commit to zthxxx/changesets that referenced this issue Dec 19, 2021
Andarist added a commit that referenced this issue Dec 21, 2021
* Make cli log message clear, reduce noise "boxen" style, unify visual style of text font, discussion in #560

* Tweak changeset

* Fix linting problems

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
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 a pull request may close this issue.

3 participants