-
Notifications
You must be signed in to change notification settings - Fork 868
Add tool for generating the DevConfig file #3811
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
Conversation
I played around with it a bit, this is neat. Thanks for making the devconfig process easier! |
public List<Service> Services { get; } = new List<Service>(); | ||
} | ||
|
||
public class CoreChange |
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 would be useful to have backwardIncompatibilitiesToIgnore
but can be in a future version.
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.
Agreed useful but I'll leave for a future version as we see how useful this tool is.
writer.WriteStartObject("core"); | ||
|
||
writer.WriteBoolean("updateMinimum", config.Core.UpdateServiceMinimum); | ||
writer.WriteString("type", config.Core.Type); |
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.
Should validate Type being either minor
or patch
in the future. Any other value isn't technically supported right now for basic DevConfigs.
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.
Done
|
||
void GeneratorChange() | ||
{ | ||
AnsiConsole.Markup("Generator changes are saved as [green]Core changes[/] with update service minimum set to [green]true[/].\n"); |
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.
A generator change doesn't always means a core update is needed. Usually it requires a change to a specific service unless it impacts many of them.
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.
Agreed but in those cases a user doesn't have to choose the generator option and could just choose the affect service. I had this option because it felt like the safe way to handle generator changes and avoid people making generator changes that did affect services they didn't realize and not get released.
var service = devConfig.Services.FirstOrDefault(x => x.Name == serviceName); | ||
if (service == null) | ||
{ | ||
service = new Service { Name = serviceName }; |
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.
When would this case be valid? It seems like it would either be a mistake name entered that has no match or trying to specify a new service which would have a folder name. Looks like this could lead to invalid devconfigs.
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.
This is a check to see if this is the first time the service has been added to the dev config file. If I add a second change log entry for a service I already add I want the same service add to the list of change services. The first time a change log entry is added for a service this will be null and added to the list of changed services.
This isn't adding a new service to our list of supported service or anything. I added a comment to make it more clear.
Description
I got tired of manually creating DevConfig files or explaining to contributors how to create a DevConfig file so I thought I add a console tool build tools for helping generating the DevConfig file would be helpful. At least it would give a head start on the file and users could later further modify the generated file.