client: mobile: accept config content#411
Open
kp-antonio-yang wants to merge 4 commits intomainfrom
Open
Conversation
Pass field meta information of json schema to client, such that mobile clients can based on the meta information to show the field really works in mobile client - pass textarea format for larger String field - pass password format for credentials String field - pass x-cfg information - add example description for durations and socket address fields
kp-antonio-yang
changed the title
|
Code coverage summary for 41aea4c: ✅ Region coverage 69% passes |
kp-antonio-yang
force-pushed
the
feat/mobile-read-conf
branch
4 times, most recently
from
c17bbd1 to
1a1e2b1
Compare
By adding a config_content input, the mobile will easy to apply yaml configs as the same as cli client using, such that we can easy to treat all clients in a unify way, and also the OSS client can directly use dynamic UI, such that the overall maintenance efforts will be less.
The build and clean command in dev shell will always run on project root.
Remove rust prefix, because we do not have benefit from this, and it will still good to use in mobile client without issues.
kp-antonio-yang
force-pushed
the
feat/mobile-read-conf
branch
from
1a1e2b1 to
2e036f1
Compare
Comment on lines
117
to
+120
| #[error("Config Error: `{0}`")] | ||
| ConfigError(#[from] crate::config::Error), | ||
| #[error("Config Format Error: `{0}`")] | ||
| ConfigFormatError(#[from] serde_saphyr::Error), |
Contributor
There was a problem hiding this comment.
What's the difference between these 2 errors?
Contributor
Author
There was a problem hiding this comment.
ConfigFormatErrorcovers errors related to the format itself — for example, invalid indentation in YAML or unclosed{}in JSON(if we accept multiple style config input).ConfigErrorcovers errors related to the value content — these are more domain-specific.
In short, ConfigFormatError is about structural/syntactic issues, while ConfigError is about semantic/domain validation.
Comment on lines
+29
to
+31
| // NOTE for JsonSchema | ||
| // - Use feature gate not cfg, such that cli client can generate schema based | ||
| // on features for other target if needed |
Contributor
There was a problem hiding this comment.
Can you share more about this? It has always been possible to cross compile to other targets
Contributor
Author
There was a problem hiding this comment.
A Windows/macOS GUI client can still be cross-compiled from a Linux machine. In that case, the developer would run the client on Linux to generate the JSON schema, which would then be used for the Windows/macOS build. This is just a note to keep in mind for future cross-compilation scenarios.
Contributor
There was a problem hiding this comment.
Okay, but can you share more about why we should use feature gate instead of cfg? Do you mean we shouldn't use Rust's conditional compilation based on platform, but use cargo features for platform specific configs?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR splits the mobile interface commits out from #391, allowing the mobile project to iterate independently without being blocked by the ongoing internal config unification work.
Description
EventHandlers, which is also an interface in KoltlinMotivation and Context
By decoupling the mobile project from the lightway project, the mobile client can move faster and more easily adopt a dynamic UI.
How Has This Been Tested?
Following existing tests.
Types of changes
Checklist:
main