Skip to content

Conversation

jeddy3
Copy link
Member

@jeddy3 jeddy3 commented May 18, 2021

Which issue, if any, is this issue related to?

Closes #5278

Is there anything in the PR that needs further explanation?

Just tying up a few loose ends.

Copy link
Member

@mattxwang mattxwang left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

I think it so good to add the explanations for each formatter. 👏🏼
But, one sentence paragraph seems a bit hard for me to read visually.

Does it make sense to add the explanations to the list as follows?

- `string` (default For CLI) - generates human-readable strings.
- `tap` - generates [Test Anything Protocol](http://testanything.org/) output.

@jeddy3
Copy link
Member Author

jeddy3 commented May 19, 2021

Much neater suggestion. It also highlighted that I'd forgotten about the compact formatter. I've pushed a commit.

Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

@jeddy3 Thank you. This improvement should be so helpful for everyone! 🙌🏼

@jeddy3 jeddy3 merged commit d540d78 into v14 May 19, 2021
@jeddy3 jeddy3 deleted the document-formatters branch May 19, 2021 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants