feat: Introduce log and refactor logic of cmd

[pimielowski]

Ultimately we agreed to not splitting the logic for the mktp and mksdk to two different files, rather to refactor the base code and introduce the log instead of io.writer.

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes if appropriate.
• All new and existing tests passed.

[shinmog]

Generally speaking, as long as there is a command that does both "create the sdk and then create the terraform provider" then I am neutral on splitting things into separate files. I think it makes sense that if there will be separate commands than the command that does both is not called "mktp," so whatever name you want to use is fine. Please put the command in the "cmd" directory with the other commands.

The reason that I don't use "log" to output log messages right now is because of unit testing. Catching and testing the content of log messages that are output using "log" is harder than checking what's written to os.Stderr / os.Stdout. In a testing scenario, you can import "os" and then replace what's saved in os.Stderr / os.Stdout with string buffers, which will allow you to validate log messages written out are as expected.

[sebastianczech]

I merged #17 into main branch, so please resolve conflicts.

Regarding splitting command at this stage of the project - I'm not convinced. Maybe later, but for now we have much more other stuff to implement.

Regarding log messages - I prefer to use log package, because the logger writes to standard error and prints the date and time of each logged message. It has also a nice feature for new lines - if the message being printed does not end in a newline, the logger will add one.

When we take final decision about splitting and logs, I will approve that PR.

[pimielowski]

Generally speaking, as long as there is a command that does both "create the sdk and then create the terraform provider" then I am neutral on splitting things into separate files. I think it makes sense that if there will be separate commands than the command that does both is not called "mktp," so whatever name you want to use is fine. Please put the command in the "cmd" directory with the other commands.

The reason that I don't use "log" to output log messages right now is because of unit testing. Catching and testing the content of log messages that are output using "log" is harder than checking what's written to os.Stderr / os.Stdout. In a testing scenario, you can import "os" and then replace what's saved in os.Stderr / os.Stdout with string buffers, which will allow you to validate log messages written out are as expected.

I redo some part of the code, and create one file in cmd/main.go to handle the invocation of the script, also I change a little bit the logic for executing the codegen for sdk and terraform, but ultimately I stick with one code file.

About the test, right now we assume if error exist it is bad :) joke aside, right now we don't have the test related to different error messages. There are couple nice libraries also introducing more capability for testing from go-kit library, like level etc, which will be nice to have, but still this is another dependencies.

[pimielowski]

I merged #17 into main branch, so please resolve conflicts.

Regarding splitting command at this stage of the project - I'm not convinced. Maybe later, but for now we have much more other stuff to implement.

Regarding log messages - I prefer to use log package, because the logger writes to standard error and prints the date and time of each logged message. It has also a nice feature for new lines - if the message being printed does not end in a newline, the logger will add one.

When we take final decision about splitting and logs, I will approve that PR.

Thanks, I merged the changes :)
I'm agreed, after some testing it doesn't have any purposes right now, so I stick to the one file.
Totally agreed about log!

[shinmog]

I have a strong preference for the ./cmd directory to only have other directories, not go code. You mentioned that you reviewed a few other big projects in our last meeting tho, did some of them do this, and you're modeling this after what you found in your investigations?

[pimielowski]

Yes, ie. Prometheus are constructed that way -> https://github.com/prometheus/prometheus/blob/main/cmd/prometheus/main.go

[pimielowski]

I change it a little bit, I added directory there

[shinmog]

I have a preference to not use log, but instead have the commands take a stdin / stderr / stdout, and then use fmt.Fprintf() to output messages there, as this setup provides easier access to the log messages output for unit tests / integration tests. If you look at the scm generator, although I don't have any unit tests there, this is modeled in that generator.

[pimielowski]

But testing is based on the function working correctly and not on function logs. We are expecting that function behave and gives us wanted output :)

[sebastianczech]

In my opinion we can go wit log approach and in tests focus on function outputs, not logs generated by function.

If in future that decision will not work for us, we can always change it :)

[sebastianczech]

👍