-
Notifications
You must be signed in to change notification settings - Fork 112
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
added support for namemaps on control properties #725
Conversation
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'd hold off on getting the schema for this approved first. but my comments are relevant none theless.
Properties: | ||
Items: |- | ||
=[{Id: 1, Title: "Item 1", Desc: "desc1"}, {Id: 2, Title: "Item 2", Desc: "desc2"}, {Id: 3, Title: "Item 3", Desc: "desc3"}] | ||
PropertiesNameMaps: |
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.
another proposal would be to just use NameMaps
, we can probably elide the 'Properties' prefix. But lets wait for a review on the schema before changing the code.
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.
PropertiesNameMaps
makes it clear that we're referring to additional metadata to enrich existing properties
src/Persistence.Tests/PaYaml/Serialization/PaYamlSerializerTests.cs
Outdated
Show resolved
Hide resolved
Properties: | ||
Items: |- | ||
=[{Id: 1, Title: "Item 1", Desc: "desc1"}, {Id: 2, Title: "Item 2", Desc: "desc2"}, {Id: 3, Title: "Item 3", Desc: "desc3"}] | ||
PropertiesNameMaps: |
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.
Open this file in VS Code. You'll see there are some squigglies due to this property not existing in the schema.
Whenever changing the schema, we should update the schema file here: src\schemas\pa-yaml\v3.0\pa.schema.yaml
I can talk with you offline how best to test changes in your inner-dev-loop.
After updating that file sufficiently, then you run src\schemas\publish.cmd so it compiles and updates the final file at schemas\pa-yaml\v3.0\pa.schema.yaml.
abandoning this PR, we have agreed on moving forward with a different schema to represent name maps:
|
added new
PropertiesNameMaps
property on theControlInstance
class