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
Support write to local file instead of post on github #654
Support write to local file instead of post on github #654
Conversation
Thank you for your contribution! |
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 package name localFile
isn't good.
https://go.dev/blog/package-names
They are lower case, with no under_scores or mixedCaps.
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 isn't bad to implement Notifier for local file, but I wonder if there is better approach. 🤔
Maintaining both github and local file isn't desirable.
force output_file to be explicitly used by command line argument. Co-authored-by: Shunsuke Suzuki <suzuki-shunsuke@users.noreply.github.com>
Co-authored-by: Shunsuke Suzuki <suzuki-shunsuke@users.noreply.github.com>
I also wondered about it, but I wanted to reuse as much existing code as possible without having to refactor everything. In the "controller.go" file, we return "client.Notify", so I wanted to keep that intact. That's why I decided to create a new notifier. Do you have a better suggestion? |
I see. Thank you for your explanation. As you said, we shouldn't change existing code as much as possible in this pull request. |
I'm on it. |
Using underscores in package names is not allowed, so I renamed "local_file" to "tofile". I find "tofile" to be a more preferable name than "localfile". What is your opinion on this? |
I prefer |
Uh... Maybe my force push ? I've amended a commit yesterday. |
Have you try it via actions tab ? |
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.
Thanks!
#194
Why
I have also the requirement to obtain the results of the Terraform plan/apply parsed and nicely formatted by tfcmt. So I am trying (for the first time) to contribute to this enhancement with my proposal.
The old method is still fully functional, I have just added a new parameter "output" to specify a file to write the result to.
To use it :
Simply use it same way before but now we can do this :
plan
apply
I attempted to preserve the essence of what I saw in the code.
I hope it will be useful and accurate... Don't hesitate to provide feedback as this is my first time working with Go.