-
Notifications
You must be signed in to change notification settings - Fork 3
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
[#254] Revise dump-config command #257
Conversation
353491c
to
510eedf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
Still leaving a few suggestions.
@@ -130,6 +130,7 @@ executables: | |||
dependencies: | |||
- xrefcheck | |||
- universum | |||
- directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mm, I wanted to ask you to follow the alphabetical order here, but it's really hard to follow 😒
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't worry. I will sort dependencies
and default-extensions
in a separate commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thank you.
There is a typo in the 8edffa2 commit's subject, but other than that looks good.
Problem: xrefcheck does not allow to print the config to stdout instead of writing it to a file. Also, it is easy to overwrite your changes by mistake by executing the command again. Solution: provide a --stdout flag to print the config to stdout, and do not write it to a file unless a --force flag has been included.
Problem: some arrays from the package.yaml file seemed to be almost alphabetically sorted, but not completely. Solution: sort the default-extensions and dependencies arrays from the package.yaml file.
8edffa2
to
1c44c1c
Compare
15264ee
to
c9486e7
Compare
This PR is quite small, I think let's merge it. |
Description
Problem: xrefcheck does not allow to print the config to stdout instead of writing it to a file. Also, it is easy to overwrite your changes by mistake by executing the command again.
Solution: provide a
--stdout
flag to print the config to stdout, and do not write it to a file unless a--force
flag has been included.Related issue(s)
Fixes #254
✅ Checklist for your Pull Request
Ideally a PR has all of the checkmarks set.
If something in this list is irrelevant to your PR, you should still set this
checkmark indicating that you are sure it is dealt with (be that by irrelevance).
Related changes (conditional)
Tests
silently reappearing again.
Documentation
Public contracts
of Public Contracts policy.
and
Stylistic guide (mandatory)