Overhaul CLI documentation and aesthetics #2703

ThomasLaPiana · 2023-02-25T06:13:04Z

Closes #2702
Closes #2704

Code Changes

Steps to Confirm

autogenerated CLI docs still work
no visual wonkiness is detected
run fides view credentials, confirm failure (no file)
run fides user login, do the thing, then fides view credentials and verify the output

Pre-Merge Checklist

All CI Pipelines Succeeded
Documentation:
- documentation complete, PR opened in fidesdocs
- documentation issue created in fidesdocs
Issue Requirements are Met
Relevant Follow-Up Issues Created
Update CHANGELOG.md

Description Of Changes

The aesthetics of the UI did not spark joy, so the rich_click package was added as a drop-in replacement to click that gives us great aesthetics for free while also being highly customizable.

An important note is that as part of this change, all help text rendered by the CLI will be assumed to be Markdown

cypress · 2023-02-25T06:26:58Z

Passing run #628 ↗︎

0	3	0	0	0

Details:

Merge `ad2f7dc` into `26e9091`...
Project: fides	Commit: `534638898a ℹ️`
Status: Passed	Duration: 00:36 💡
Started: Mar 6, 2023 5:13 AM	Ended: Mar 6, 2023 5:14 AM

_{This comment has been generated by cypress-bot as a result of this
project's GitHub integration settings.}

codecov · 2023-02-25T06:36:57Z

Codecov Report

Patch coverage: 95.61% and project coverage change: +0.14 🎉

Comparison is base (26e9091) 86.60% compared to head (ad2f7dc) 86.75%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2703      +/-   ##
==========================================
+ Coverage   86.60%   86.75%   +0.14%     
==========================================
  Files         290      291       +1     
  Lines       16242    16312      +70     
  Branches     2065     2066       +1     
==========================================
+ Hits        14066    14151      +85     
+ Misses       1793     1777      -16     
- Partials      383      384       +1

Impacted Files	Coverage Δ
src/fides/cli/commands/view.py	`83.87% <81.25%> (-5.02%)`	⬇️
src/fides/cli/commands/crud.py	`95.23% <83.33%> (+1.12%)`	⬆️
src/fides/cli/__init__.py	`92.15% <100.00%> (+0.15%)`	⬆️
src/fides/cli/cli_formatting.py	`100.00% <100.00%> (ø)`
src/fides/cli/commands/annotate.py	`86.66% <100.00%> (ø)`
src/fides/cli/commands/core.py	`92.06% <100.00%> (ø)`
src/fides/cli/commands/db.py	`81.48% <100.00%> (ø)`
src/fides/cli/commands/export.py	`100.00% <100.00%> (ø)`
src/fides/cli/commands/generate.py	`86.36% <100.00%> (ø)`
src/fides/cli/commands/scan.py	`88.46% <100.00%> (ø)`
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

ThomasLaPiana · 2023-02-25T12:22:02Z

little demo of what this looks like

…options

ThomasLaPiana · 2023-03-01T01:50:05Z

@SteveDMurphy do the export commands need to have the dry flag other than the datamap export?

SteveDMurphy · 2023-03-01T07:51:36Z

@SteveDMurphy do the export commands need to have the dry flag other than the datamap export?

They were primarily added for running tests against the commands, we can probably revamp those to either check a temp directory that the file exists if we want to get rid of them? (overlap with ls now I imagine)

Separately, @sanders41 and I were discussing the value of keeping all of the export commands, potentially removing them as something else to maintain and being able to simplify the export datamap + custom fields stuff. Would be curious to hear your thoughts too!

pattisdr

CLI docs formatting is 💯 . Just noted minor things as I experimented.

src/fides/cli/__init__.py

src/fides/cli/commands/export.py

src/fides/cli/commands/generate.py

src/fides/cli/commands/view.py

src/fides/cli/__init__.py

src/fides/cli/options.py

TheAndrewJackson

I like these updates. Much more concise throughout. It's awesome that you're able to use markdown in the help copy. I'm not used to seeing it in the terminal.

I found one instance of missing markdown formatting. Picture below:

src/fides/cli/__init__.py

src/fides/cli/commands/generate.py

src/fides/cli/options.py

ThomasLaPiana · 2023-03-02T06:39:15Z

@SteveDMurphy do the export commands need to have the dry flag other than the datamap export?

They were primarily added for running tests against the commands, we can probably revamp those to either check a temp directory that the file exists if we want to get rid of them? (overlap with ls now I imagine)

Separately, @sanders41 and I were discussing the value of keeping all of the export commands, potentially removing them as something else to maintain and being able to simplify the export datamap + custom fields stuff. Would be curious to hear your thoughts too!

I have thoughts on the entire CLI design, but export did stick out as something that confused me, which means there's a high likelihood it would confuse users too. I'll follow this up in the other issue for CLI design #2718

ThomasLaPiana · 2023-03-02T08:18:17Z

Patch coverage is down but overall coverage is up. Testing the remaining patch pieces is not worth it from a time perspective (and complexity of testing those specific pieces) in my opinion

Update requirements.txt

19d8169

ThomasLaPiana self-assigned this Feb 25, 2023

feat: add rich_click and play around with formatting

d973264

ThomasLaPiana added 2 commits February 25, 2023 20:42

feat: add more formatting, update docstrings

9878d90

docs: more docstring updates

466b6da

ThomasLaPiana changed the title ~~Replace click with `rich_click~~ Replace click with rich_click Feb 25, 2023

ThomasLaPiana added 2 commits February 26, 2023 02:23

docs: more broad changes

47ac9ee

docs: update annotate group documentation

b9e9dab

ThomasLaPiana changed the title ~~Replace click with rich_click~~ Overhaul CLI documentation and aesthetics Feb 25, 2023

ThomasLaPiana added 4 commits February 26, 2023 02:49

docs: db commands

dfaedaf

feat: clean up the CRUD command documentation as well as outputs and …

75dd0a4

…options

docs: slight deploy verbiage tweak

1e59a03

docs: update evaluate command

1143b3e

docs: export and generate subcommands

e1b0f89

ThomasLaPiana added 10 commits March 1, 2023 19:28

Merge branch 'main' into ThomasLaPiana-add-rich-click

832e857

docs: more docstring updates

91e4298

feat: add a fides view credentials command

c8a92cc

docs: options and scan commands

26b1ad8

docs: change datamap -> data map everywhere

c781c80

fix: errant find/replace

31741ed

docs: slight docstring tweak

f0ef506

docs: changelog

61f63f9

fix: resource listing and static checks

42ced75

fix: mypy and stuff

667c622

ThomasLaPiana requested a review from SteveDMurphy March 1, 2023 19:49

ThomasLaPiana requested review from pattisdr and TheAndrewJackson March 1, 2023 19:49

ThomasLaPiana added 2 commits March 2, 2023 04:09

fix: typo

a26adf6

fix: consistent "data map"

a0df6ae

ThomasLaPiana requested a review from NevilleS March 1, 2023 20:11

pattisdr reviewed Mar 1, 2023

View reviewed changes

TheAndrewJackson reviewed Mar 2, 2023

View reviewed changes

src/fides/cli/__init__.py Show resolved Hide resolved

src/fides/cli/commands/generate.py Outdated Show resolved Hide resolved

src/fides/cli/options.py Outdated Show resolved Hide resolved

ThomasLaPiana added 2 commits March 2, 2023 13:27

fix: fides deploy check failure

1616b68

fix: review comments

e1e246a

ThomasLaPiana marked this pull request as ready for review March 2, 2023 05:33

ThomasLaPiana added 2 commits March 2, 2023 15:06

fix: test coverage and docstring

fe4da8f

fix: test the verbose option

84af3c4

ThomasLaPiana requested review from pattisdr and TheAndrewJackson March 2, 2023 08:19

ThomasLaPiana changed the title ~~Overhaul CLI documentation and aesthetics~~ [Do Not Merge] Overhaul CLI documentation and aesthetics Mar 2, 2023

ThomasLaPiana changed the title ~~[Do Not Merge] Overhaul CLI documentation and aesthetics~~ Overhaul CLI documentation and aesthetics Mar 3, 2023

pattisdr approved these changes Mar 5, 2023

View reviewed changes

Merge branch 'main' into ThomasLaPiana-add-rich-click

ad2f7dc

ThomasLaPiana merged commit 987b05b into main Mar 6, 2023

ThomasLaPiana deleted the ThomasLaPiana-add-rich-click branch March 6, 2023 05:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Overhaul CLI documentation and aesthetics #2703

Overhaul CLI documentation and aesthetics #2703

ThomasLaPiana commented Feb 25, 2023 •

edited

Loading

cypress bot commented Feb 25, 2023 •

edited

Loading

codecov bot commented Feb 25, 2023 •

edited

Loading

ThomasLaPiana commented Feb 25, 2023 •

edited

Loading

ThomasLaPiana commented Mar 1, 2023

SteveDMurphy commented Mar 1, 2023

pattisdr left a comment

TheAndrewJackson left a comment

ThomasLaPiana commented Mar 2, 2023

ThomasLaPiana commented Mar 2, 2023 •

edited

Loading

Overhaul CLI documentation and aesthetics #2703

Overhaul CLI documentation and aesthetics #2703

Conversation

ThomasLaPiana commented Feb 25, 2023 • edited Loading

Code Changes

Steps to Confirm

Pre-Merge Checklist

Description Of Changes

cypress bot commented Feb 25, 2023 • edited Loading

Passing run #628 ↗︎

codecov bot commented Feb 25, 2023 • edited Loading

Codecov Report

ThomasLaPiana commented Feb 25, 2023 • edited Loading

ThomasLaPiana commented Mar 1, 2023

SteveDMurphy commented Mar 1, 2023

pattisdr left a comment

Choose a reason for hiding this comment

TheAndrewJackson left a comment

Choose a reason for hiding this comment

ThomasLaPiana commented Mar 2, 2023

ThomasLaPiana commented Mar 2, 2023 • edited Loading

ThomasLaPiana commented Feb 25, 2023 •

edited

Loading

cypress bot commented Feb 25, 2023 •

edited

Loading

codecov bot commented Feb 25, 2023 •

edited

Loading

ThomasLaPiana commented Feb 25, 2023 •

edited

Loading

ThomasLaPiana commented Mar 2, 2023 •

edited

Loading