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.

Sign up for GitHub

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

Jump to bottom

Remove unclear config warning #2266

Merged
merged 3 commits into from
Jan 19, 2023
Merged

Remove unclear config warning #2266

merged 3 commits into from
Jan 19, 2023

Conversation

ThomasLaPiana
Copy link
Contributor

@ThomasLaPiana ThomasLaPiana commented Jan 18, 2023 •
edited
Loading

Closes #2164

Code Changes

  • remove the unclear message
  • move the config_path logging message to within the try block

Steps to Confirm

  • move into a directory without a .fides/fides.toml
  • run fides view config
  • note that the logs show Using default configuration values.

Pre-Merge Checklist

Description Of Changes

This is a quick PR to make one of the config-related logging messages less scary/confusing.

Old

test> fides view config
No config file found.
Using default configuration values.

New

test> fides view config
Using default configuration values.

@ThomasLaPiana ThomasLaPiana self-assigned this Jan 18, 2023
@ThomasLaPiana ThomasLaPiana marked this pull request as ready for review January 18, 2023 11:32
pattisdr
pattisdr approved these changes Jan 18, 2023
View reviewed changes
Copy link
Contributor

@pattisdr pattisdr left a comment

Choose a reason for hiding this comment

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

Good path forward here

TheAndrewJackson
TheAndrewJackson approved these changes Jan 18, 2023
View reviewed changes
Copy link
Contributor

@TheAndrewJackson TheAndrewJackson left a comment

Choose a reason for hiding this comment

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

:shipit:

@ThomasLaPiana
Copy link
Contributor Author

Good path forward here

i see what you did there, :1000:/💯

@ThomasLaPiana ThomasLaPiana merged commit 00cff5e into main Jan 19, 2023
@ThomasLaPiana ThomasLaPiana deleted the ThomasLaPiana-remove-unclear-config-warning branch January 19, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheAndrewJackson TheAndrewJackson TheAndrewJackson approved these changes

@pattisdr pattisdr pattisdr approved these changes

@allisonking allisonking Awaiting requested review from allisonking

Assignees

@ThomasLaPiana ThomasLaPiana

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The no config file found message is unclear for new users
3 participants
@ThomasLaPiana @pattisdr @TheAndrewJackson