-
Notifications
You must be signed in to change notification settings - Fork 54
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
Refactor tedge-mapper by using new tedge config API #2013
Refactor tedge-mapper by using new tedge config API #2013
Conversation
Robot Results
Passed 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.
I think we need to restrict the use of ReadableKey
to the tedge_config
crate as the point of this new tedge config is to free the developers from the burden of maintaining the relationship between path in tedge_config.toml
with SettingXYZ
or KeyXYZ
.
So create methods in NewTedgeConfig
for the keys that cannot be accessed directly (because private or computed). For instance, NewTEdgeConfig::c8y_host()
for ReadableKey::C8yHttp
.
For setting that can be accessed directly , use the natural syntax, e.g. tedge_config.tmp.path
.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2013 +/- ##
==========================================
+ Coverage 72.59% 73.23% +0.64%
==========================================
Files 279 281 +2
Lines 30630 30966 +336
Branches 30630 30966 +336
==========================================
+ Hits 22236 22679 +443
+ Misses 6796 6651 -145
- Partials 1598 1636 +38
☔ View full report in Codecov by Sentry. |
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.
Approved.
Signed-off-by: Rina Fujino <18257209+rina23q@users.noreply.github.com>
fdd383f
to
a45f436
Compare
Proposed changes
For #1969, I need to add new tedge config keys to the new tedge config API in order to use them in tedge-mapper. However, to proceed it, firstly I need to refactor tedge-mapper by replacing the old tedge config API by the new one. Otherwise, it's impossible to load the new API keys from the old tedge config API object.
This PR is compilable, but there is still questionable point if I used the new tedge config API in a designed manner. Feel free to add comments!
Types of changes
Paste Link to the issue
Part of #1969
Checklist
cargo fmt
as mentioned in CODING_GUIDELINEScargo clippy
as mentioned in CODING_GUIDELINESFurther comments