-
Notifications
You must be signed in to change notification settings - Fork 1
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
Update schema code generation code to reflect manually made changes #128
Conversation
0a13221
to
059571d
Compare
e428dbf
to
ddf89b0
Compare
fa6ffde
to
92af399
Compare
8e94fff
to
a53b103
Compare
2fafab2
to
521aac2
Compare
…se in the json property attribute. Adds some tests & documentation
8d33644
to
e77176b
Compare
@@ -5,6 +5,12 @@ namespace Btms.Backend.Cli.Features.GenerateModels.ClassMaps; | |||
|
|||
internal static class Bootstrap | |||
{ | |||
//Not ideal that the casing doesn't match, but the coding styles mandate: |
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.
What's the significance of this code block and what is not ideal? If it's a worthy thing to comment on then might be worth enhancing the detail or it's just for string replacement/reuse within the bootstrapping process
|
|
"commandName": "Project", | ||
"commandLineArgs": "generate-ipaffs-model" | ||
}, | ||
"Other": { | ||
"commandName": "Project", |
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.
Can we remove this "Other" block now? And if not, can we take what we need and formalise specific run profiles as you have been doing as that is a better approach.
public DateTime? DecisionsValidUntil { get; set; } | ||
|
||
|
||
/// <summary> |
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.
We need this blank summary removing at some point too and potentially not adding the [System.ComponentModel.Description("")]
when there is no value. Unless that attribute needs to be there all the time regardless for some reason?
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.
Ah - think that may have been manually added - should be removed
/// <summary> | ||
/// | ||
/// </summary> | ||
[JsonPropertyName("decisionNumber")] |
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.
Why did this field move from a .g.cs
file to a non generated one?
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.
It's not - it's not in the schema so is manually added
Has an issue with formatting of code, similar to what it was before. Will revisit this separately.