Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Return error code when issue exists #31

Merged
merged 1 commit into from Jan 9, 2017
Merged

Return error code when issue exists #31

merged 1 commit into from Jan 9, 2017

Conversation

ytet5uy4
Copy link
Contributor

@ytet5uy4 ytet5uy4 commented Jan 6, 2017

WHY

When used tflint on CI, it's didn't failed although tflint return some errors.

WHAT

Update to return 2 when issue exists.

@wata727
Copy link
Member

wata727 commented Jan 6, 2017

Thanks for your proposal.
However, I think that it is not desirable to return the same status code as it was when an error occurred in core. Perhaps, it should return status code for that reason.

In my opinion, status code returned by lint tools should be customizable. We should be able to control it by runtime arguments as follows without changing the default behavior.

$ tflint
# return exit status code is 0 

$ tflint --exit-error-status
# return exit status code is something that is neither 0 nor 1

@ytet5uy4 ytet5uy4 changed the title Return error code when issue exists [WIP] Return error code when issue exists Jan 8, 2017
@ytet5uy4
Copy link
Contributor Author

ytet5uy4 commented Jan 8, 2017

I've updated to match your opinion.
Is my understanding correct?

wata727
wata727 reviewed Jan 8, 2017
View reviewed changes
cli.go
@@ -148,6 +150,12 @@ func (cli *CLI) Run(args []string) int {

p := printer.NewPrinter(cli.outStream, cli.errStream)
p.Print(issues, format)

if exitErrorStatus {
if len(issues) > 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpicks] if exitErrorStatus && len(issues) > 0

cli.go
@@ -71,6 +72,7 @@ func (cli *CLI) Run(args []string) int {
flags.StringVar(&awsAccessKey, "aws-access-key", "", "AWS access key used in deep check mode.")
flags.StringVar(&awsSecretKey, "aws-secret-key", "", "AWS secret key used in deep check mode.")
flags.StringVar(&awsRegion, "aws-region", "", "AWS region used in deep check mode.")
flags.BoolVar(&exitErrorStatus, "exit-error-status", false, "return exit status code when issue exists.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry... Please use another option name 🙇
I think --exit-error-status is bad name. (My recommendation is --error-with-issues)

cli.go

if exitErrorStatus {
if len(issues) > 0 {
return ExitCodeError
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use another status code. (ex: ExitCodeIssuesFound, etc.) ExitCodeError is reserved for execution error.

@ytet5uy4 ytet5uy4 changed the title [WIP] Return error code when issue exists Return error code when issue exists Jan 8, 2017
@wata727
Copy link
Member

wata727 commented Jan 8, 2017

Great! Finally, Please squash to one commit as well :)

@wata727 wata727 merged commit 430deb7 into terraform-linters:master Jan 9, 2017
@wata727
Copy link
Member

wata727 commented Jan 9, 2017

Thanks! I will release the next version including this PR soon. Please give me feedback again!

bendrucker pushed a commit that referenced this pull request Jun 13, 2023
@steven-terrana
Fix typo in configuration.md (#31)
0c3dad9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wata727 wata727 wata727 left review comments

Assignees
No one assigned
Labels
enhancement
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ytet5uy4 @wata727