-
-
Notifications
You must be signed in to change notification settings - Fork 386
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
[BREAKING] gRPC / golang Configuration API refactoring #2565
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2565 +/- ##
==========================================
+ Coverage 69.51% 70.13% +0.61%
==========================================
Files 207 222 +15
Lines 20285 21124 +839
==========================================
+ Hits 14102 14816 +714
- Misses 5069 5132 +63
- Partials 1114 1176 +62
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
7d99eb4
to
99645aa
Compare
c92db6a
to
593326d
Compare
e1583fc
to
149c7e1
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.
LGTM
I'd say after the PR that unifies the usage of oneOf
in our API, we can schedule a round of manual testing to catch any regression (if any) not seen by our integration tests.
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.
Overall LGTM, I did not find any major problem.
Should we update also this: https://github.com/arduino/arduino-cli/blob/master/docs/integration-options.md ?
Yes, BTW I'm doing it in another branch as part of a general revamp of the documentation. |
Please check if the PR fulfills these requirements
See how to contribute
before creating one)
our contributing guidelines
UPGRADING.md
has been updated with a migration guide (for breaking changes)configuration.schema.json
updated if new parameters are added.What kind of change does this PR introduce?
This PR introduces a big change in the configuration API, the main goals for this PR are:
What is the current behavior?
There should be very few changes in the CLI behavior, in theory, final users should not notice any change.
What is the new behavior?
Does this PR introduce a breaking change, and is titled accordingly?
Yes, the gRPC Settings* API changed a lot, adaption on the client is required.
Other information