Skip to content
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

Fix Model Conversion #35

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Sefriol
Copy link

@Sefriol Sefriol commented Oct 5, 2023

  • Comments were overwriting EnumValues. I do not think this should be the case. Atleast it is not documented DTDL language specification
  • Current generator assumes that every model starts with uppercase letters. This is not always the case. This change ensures that classes/modules are written and then referenced with uppercase letters.

@Sefriol Sefriol requested a review from a team as a code owner October 5, 2023 08:33
@jarz
Copy link
Member

jarz commented Oct 5, 2023

@ms-mikeb: I can't recall the (historic?) reason for the SourceValue behavior with enum comments. Thoughts?

@Sefriol
Copy link
Author

Sefriol commented Oct 12, 2023

@jarz Any updates? I can iterate this if need be.

@ms-mikeb
Copy link
Collaborator

@ms-mikeb: I can't recall the (historic?) reason for the SourceValue behavior with enum comments. Thoughts?

Sorry for the late reply on this. We have an old system we needed to be able to map the value we're storing in the Comment field to an attribute that's added to the model in C#. I recognize this behavior wasn't documented well, nor is it intuitive. I'll need to see what alternatives we can look at for managing this so we can retire this behavior.

@ms-mikeb
Copy link
Collaborator

ms-mikeb commented Oct 25, 2023

@jarz Any updates? I can iterate this if need be.

@Sefriol

I would say, let's make an issue to track the Comment, and just focus the PR on the casing support.

@ms-mikeb ms-mikeb added the type:bug Bug fix of existing functionality label Oct 25, 2023
@Sefriol Sefriol force-pushed the fix/model-convertion branch from 5930251 to 69b6508 Compare October 26, 2023 13:05
@Sefriol
Copy link
Author

Sefriol commented Oct 26, 2023

Changed the logic so that Comment takes precedence over enumvalue. There were some other issues with enum property names that I wanted to fix as well, so I kept those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Bug fix of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants