Added support for List of Array examples. #177
Conversation
| @@ -410,6 +410,14 @@ pub enum Examples { | |||
| Bools(Vec<bool>), | |||
| /// A array of strings example. | |||
| Strings(Vec<String>), | |||
| /// List of arrays of integers example. | |||
| ListOfInts(Vec<Vec<i64>>), | |||
There was a problem hiding this comment.
Could you add a test to ensure this is working?
There was a problem hiding this comment.
I've added tests... feels like I'm just testing serde_yaml though ;)
There was a problem hiding this comment.
Thanks, and agreed. I had intended for those tests to be on full resolution ensuring these are preserved through the resolution process, but what you have is fine for now.
There was a problem hiding this comment.
Overall, it LGTM, but the [!NOTE] tags need to be put back in place because they are interpreted by GitHub in a specific way. It is possible that rustdoc or clippy might not be happy, in which case you should use a #[allow()] to suppress the warnings. Thank you for this contribution.
crates/weaver_checker/README.md
Outdated
| @@ -105,10 +105,8 @@ deny[schema_evolution_violation("attr_removed", old_group.id, old_attr.id)] { | |||
| } | |||
| ``` | |||
|
|
|||
| > [!NOTE] | |||
There was a problem hiding this comment.
These [!NOTE] are valid GitHub Markdown Extensions, see the following blog post.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #177 +/- ##
=======================================
- Coverage 75.5% 74.6% -1.0%
=======================================
Files 44 43 -1
Lines 2849 2751 -98
=======================================
- Hits 2153 2053 -100
- Misses 696 698 +2 ☔ View full report in Codecov by Sentry. |
|
@jerbly The issue with |
| @@ -410,6 +410,14 @@ pub enum Examples { | |||
| Bools(Vec<bool>), | |||
| /// A array of strings example. | |||
| Strings(Vec<String>), | |||
| /// List of arrays of integers example. | |||
| ListOfInts(Vec<Vec<i64>>), | |||
There was a problem hiding this comment.
Thanks, and agreed. I had intended for those tests to be on full resolution ensuring these are preserved through the resolution process, but what you have is fine for now.
The current implementation does not support all the Example types as defined here: https://github.com/open-telemetry/build-tools/blob/main/semantic-conventions/syntax.md#examples-for-examples - specifically multiple example values for array types.
I have added these as
ListOf*variants to theExamplesenum.The legacy markdown generation had to be updated to accommodate this, and to fix the presentation of array type examples. An example for a
string[]attribute type should be displayed as an array:["a", "b"]. In the current implementation they are shown as multiple examples appropriate forstringtype:a;b. I'm not sure where the legacy markdown writer is actually used in practice though.This error in representation of examples has also been carried through to the jinja templates in the semconv project and so currently all the published examples for array types in the documentation are incorrect. (I have a PR to fix this going up ASAP).
Not sure what the pattern is for the changelog, it appears to show all unreleased entries and yet 0.3.0 has been released?? (But the main cargo.toml has 0.1.0??)