-
-
Notifications
You must be signed in to change notification settings - Fork 423
add "config get" command to print settings values #2307
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
Conversation
Hello @ardnew! Thanks for your contribution.
Is this PR ready to be reviewed? If so, can you please rebase your branch onto arduino:master to include the latest commits and resolve any conflicts?
If you have any questions, please feel free to ask!
|
@MatteoPologruto Will try to rebase this PR very soon to fix the conflicts. Also, there were 2 items in the PR template that had not been completed (see below), which is why this was originally posted as a draft. Not sure if updating the docs is essential in this case or if I can submit the PR without it (after resolving conflicts)?
|
@ardnew don't worry, updating the docs is not needed in this case. The items you checked are the mandatory ones for this kind of contribution. I was asking if this was still a draft because I did not know if you were planning to further modify this PR. Since this is not the case, I will proceed with my review.
a0b6aaf to
47c09a8
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@ ## master #2307 +/- ## ========================================== + Coverage 68.95% 68.97% +0.01% ========================================== Files 204 204 Lines 20535 20449 -86 ========================================== - Hits 14160 14104 -56 + Misses 5220 5193 -27 + Partials 1155 1152 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
1522588 to
391a80e
Compare
Co-authored-by: Cristian Maglie <c.maglie@bug.st>
a1bf14f to
c069ece
Compare
@ardnew code LGTM 🎉
You only need to update the tests according to the changes you made. After that, I'd say we are ready to merge!
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.
Great job! Thanks for your contribution @ardnew 🎉
Uh oh!
There was an error while loading. Please reload this page.
Please check if the PR fulfills these requirements
See how to contribute
before creating one)
our contributing guidelines
(削除)UPGRADING.mdhas been updated with a migration guide (for breaking changes) (削除ここまで)(削除)configuration.schema.jsonupdated if new parameters are added. (削除ここまで)What kind of change does this PR introduce?
Feature addition
What is the current behavior?
There are
configcommands toadd/setindividual settings, but retrieving settings requires the user todumpall settings and parse the output with either JSON/YAML to retrieve an individual.What is the new behavior?
With the addition of a
config getcommand, the user can retrieve individual configuration settings without having to parse JSON/YAML.Does this PR introduce a breaking change, and is titled accordingly?
Not a breaking change
Other information
None