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

FEATURE: add a pretty output to toolkit check pr #8416

Merged

Conversation

amtoine
Copy link
Member

@amtoine amtoine commented Mar 12, 2023

when i write a PR, i run the tests and i like to have a pretty output to make extra clear which one of the tests did run, which one did not, etc, etc...

this always end up a variation of the template

  • cargo fmt --all -- --check to check standard code formatting (cargo fmt --all applies these changes)
  • cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect to check that you're using the standard code style
  • cargo test --workspace to check that all tests pass

but with emojis and without the descriptions

  • 🟢 cargo fmt --all
  • 🔴 cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect
  • 🟡 cargo test --workspace

and a ⚫ (:black_circle:) when i did not have the time or the resources to run the check stage

in this PR, i came up with a way to do that automatically with the toolkit introduced in #8152 😋

Description

this PR

  • adds toolkit::pretty-print-command to print the command names being run with backticks and some colors
  • adds toolkit::report to return a "report" of the PR check stages => see help toolkit check pr
  • adds the --pretty option to toolkit check pr to return a list-with-emojis version of the check report, i.e. a GitHub-friendly list to drop in place in the "Tests + Formatting" section
  • adds a clear mention to toolkit check pr --pretty in the PR template to make it easily visible to anyone

hope you'll like it, that's not a huge deal but that's my attempt to encourage developers to show that they run the tests, what stages did pass and which one did not 😌 👋

User-Facing Changes

the developer can now use toolkit check pr --pretty to have a ready-to-use output for GitHub

Tests + Formatting

$nothing

After Submitting

$nothing

This commit allows the developer to run
```bash
toolkit check pr --pretty
```
and get a *GitHub*-friendly output to be copy-pasted to the PR.
This is to encourage the use of a common way of showing the tests
pass
- a red circle means the stage does not pass
- a green circle means the stage passes
- a black circle means the stage did not run because of previous stages
@fdncred
Copy link
Collaborator

fdncred commented Mar 12, 2023

I'm confused, what's the pretty output look like?

@codecov
Copy link

codecov bot commented Mar 12, 2023

Codecov Report

Merging #8416 (9f972bc) into main (a52386e) will increase coverage by 0.39%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8416      +/-   ##
==========================================
+ Coverage   68.11%   68.50%   +0.39%     
==========================================
  Files         620      620              
  Lines       99709    99723      +14     
==========================================
+ Hits        67914    68317     +403     
+ Misses      31795    31406     -389     

see 8 files with indirect coverage changes

@amtoine
Copy link
Member Author

amtoine commented Mar 12, 2023

I'm confused, what's the pretty output look like?

yes of course, let me show you 👍

@amtoine
Copy link
Member Author

amtoine commented Mar 12, 2023

without the --pretty option

  • if the formatting stage fails
╭────────┬───────╮
│ fmt    │ false │
│ clippy │       │
│ test   │       │
╰────────┴───────╯
  • if the clippy stage fails
╭────────┬───────╮
│ fmt    │ true  │
│ clippy │ false │
│ test   │       │
╰────────┴───────╯
  • if the test stage fails
╭────────┬───────╮
│ fmt    │ true  │
│ clippy │ true  │
│ testfalse │
╰────────┴───────╯
  • if all the stages pass
╭────────┬──────╮
│ fmt    │ true │
│ clippy │ true │
│ testtrue │
╰────────┴──────╯

with the --pretty option

  • if the formatting stage fails
  • 🔴 toolkit fmt
  • toolkit clippy
  • toolkit test
  • if the clippy stage fails
  • 🟢 toolkit fmt
  • 🔴 toolkit clippy
  • toolkit test
  • if the test stage fails
  • 🟢 toolkit fmt
  • 🟢 toolkit clippy
  • 🔴 toolkit test
  • if all the stages pass
  • 🟢 toolkit fmt
  • 🟢 toolkit clippy
  • 🟢 toolkit test

@amtoine
Copy link
Member Author

amtoine commented Mar 12, 2023

so the --pretty options outputs the emojis to have the colored circles and an easy to understand status of the tests 👍

@@ -25,6 +25,10 @@ Make sure you've run and fixed any issues with these commands:
> use toolkit.nu # or use an `env_change` hook to activate it automatically
> toolkit check pr
> ```
> you can replace this whole "Tests + Formatting" section by the list output of
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's just make --pretty the default and not add these lines. I'd like to keep this template as abbreviated as possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

should be fixed in 9f972bc 😋

@amtoine
Copy link
Member Author

amtoine commented Mar 18, 2023

is there anything i can do here? 😋
( yes i wanted to use this new feature for a PR right now and saw it was not merged 😆 )

@fdncred fdncred merged commit c66bd5e into nushell:main Mar 18, 2023
@fdncred
Copy link
Collaborator

fdncred commented Mar 18, 2023

Lost track of it. Thanks

@amtoine
Copy link
Member Author

amtoine commented Mar 18, 2023

Lost track of it. Thanks

no worries, it's far easier for me to keep track of my own stuff 😉
i thought a friendly ping might help 😌

@amtoine amtoine deleted the feature/add-a-pretty-output-to-toolkit-check-pr branch March 18, 2023 13:02
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.

2 participants