Add allowed values to metadata #69
Conversation
argerus
force-pushed
the
feature/add_allowed_to_metadata
branch
from
edd5b04
to
4f1a0b0
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #69 +/- ##
==========================================
- Coverage 50.44% 50.02% -0.42%
==========================================
Files 31 31
Lines 11702 11805 +103
==========================================
+ Hits 5903 5906 +3
- Misses 5799 5899 +100 ☔ View full report in Codecov by Sentry. |
|
As quick fix okay. But shouldn't we add it in kuksa.val.v1/v2 as well as message in types? |
|
Should not be merged to https://github.com/boschglobal/kuksa-databroker/tree/prep-rel-0.5.0? |
This adds it to |
|
yes, in code but why only add |
|
Okay, my fault. It is nested in |
| r#type: Some(proto::value_restriction::Type::Signed( | ||
| proto::ValueRestrictionInt { | ||
| allowed_values: vec.iter().cloned().map(i64::from).collect(), | ||
| min: None, // TODO: Implement |
There was a problem hiding this comment.
So for now just creating a placeholder for supporting min/max?
There was a problem hiding this comment.
These were already defined in kuksa.val.v1.ValueRestriction.. this PR doesn't change anything regarding them, it's limited to adding support for allowed values.
But maybe it would be better to add support for min / max as well in this PR?
There was a problem hiding this comment.
I have no strong opinion, depends on how much time that would take I think. If it is quick go for it, if not keep the TODOs and do it later.
| int32 id = 1; | ||
| EntryType entry_type = 2; | ||
| string name = 4; | ||
| DataType data_type = 5; | ||
| ChangeType change_type = 6; // CONTINUOUS or STATIC or ON_CHANGE | ||
| string description = 7; | ||
|
|
||
| Allowed allowed = 10; |
There was a problem hiding this comment.
Have we ever used the metadata on the lines below? Even if the lines below are commented I assume it would be better to not have the same number (10) on this line and the line below, or?
There was a problem hiding this comment.
They have never been used. I guess it's better to just remove them.
There was a problem hiding this comment.
Then just remove them I would say
erikbosch
force-pushed
the
feature/add_allowed_to_metadata
branch
from
4f1a0b0
to
c511caa
Compare
No description provided.