-
Notifications
You must be signed in to change notification settings - Fork 64
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
Feature/add feature print configuration issue 172 #203
base: master
Are you sure you want to change the base?
Feature/add feature print configuration issue 172 #203
Conversation
- Add ConfigOption.cs and BaseConfigOption.cs files for config cmd options. - Add RunConfigOption method to handle config cmd use case. closes rdagumampan#172
Implment RunConfigurationOption method to handle config command use case. closes rdagumampan#172
- add IPrinter interface - add JsonPrinter class closes rdagumampan#172
- Choose text format for printing configuration variable information. - Add json option (by default the format is table/rows) closes closes rdagumampan#172
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.
Hi @zwdOr20, thank you very much for your interest in yuniql and contribution.
Please see my comments and resubmit changes when you are ready. Have a nice day!
@@ -10,7 +10,7 @@ public class BaseOption | |||
|
|||
//yuniql <command> -d | --debug | |||
[Option('d', "debug", Required = false, HelpText = "Print debug information including all raw scripts.")] | |||
public bool IsDebug { get; set; } | |||
public bool? IsDebug { get; set; } |
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'm curious why you think this has to be changed. I'm afraid it may introduce bugs not directly related to feature we're implementing. :)
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.
The raison why I made it nullable is I want to differentiate between two states :
- IsDebug is false && the value wasn't specified by the user (default value for boolean is False doc)
- When the user specify --debug false
In both cases IsDebug set to false but the source of this false is different
- change data-type option name to output - change Source enum values fomat to ALLCAPS - chekout yuniql-platforms/postgresql/PostgreSqlBulkImportService.cs to master.
Kudos, SonarCloud Quality Gate passed! |
Kudos, SonarCloud Quality Gate passed! |
In this PR I tried to implement a solution for the open issue issue, I'm fully open for any suggestions and enhancement !
Ps : I didn't want to write unit tests for my code before discussing the PR!
Implemented use case : yuniql config || yuniql config -o json