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.

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

Return with exit code 0 if no files are found #133

Closed
jd1378 opened this issue Jul 25, 2020 · 5 comments · Fixed by #134
Closed

Return with exit code 0 if no files are found #133

jd1378 opened this issue Jul 25, 2020 · 5 comments · Fixed by #134
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@jd1378
Copy link
Contributor

jd1378 commented Jul 25, 2020

Hi,
I'm currently trying to use this on gitlab ci, and currently don't have any files to lint, but I think since the linter exits with non zero the job fails.
isn't it better to exit with 0 when there's nothing to lint ?
here is what I use on gitlab:

protobuf-lint:
  image: 
    name: yoheimuta/protolint:v0.25.1
    entrypoint: [""]
  stage: test
  script:
    - protolint lint -v -reporter junit -output_file=junit.xml .
  artifacts:
    reports:
      junit: junit.xml
    expire_in: 1 week
@yoheimuta
Copy link
Owner

@jd1378 Thank you for your interesting suggestion.

I understand your point. Can you show me some linters that behave so?
I think it's better to follow the nearly de facto rule if it exists (because many tools depend on this). Then, if it doesn't seem to be the de facto rule, we should support it with a new option.

For example, eslint that has immense popularity exits with code 0 when it found no files.

❯❯❯ ls empty
hoge.txt
❯❯❯ eslint "empty/*"; echo $?
0

[memo] OTOH, it exits with 2 when the glob doesn't match any files. --no-error-on-unmatched-pattern option can change this behavior.

❯❯❯ rm empty/hoge.txt
remove empty/hoge.txt? y
❯❯❯ eslint "empty/*"; echo $?

Oops! Something went wrong! :(

ESLint: 7.5.0

No files matching the pattern "empty/*" were found.
Please check for typing mistakes in the pattern.

2
❯❯❯ eslint --no-error-on-unmatched-pattern true "empty/*"; echo $?
0

@jd1378
Copy link
Contributor Author

jd1378 commented Jul 25, 2020

You are right, I wasn't thinking about that
I checked out golangci-lint with golangci-lint run ./...; echo $? and it also exits with code 5 when it founds no file
I'll try to find more, but it looks like that going with a new option be the best way to do it and it won't be a breaking change if anyone by any chance depending on this

@yoheimuta
Copy link
Owner

@jd1378 Thank you for your confirmation.
I agree with you.

BTW, I added a commit f45a1c7 to clarify the current spec. In this case, it should return an error message with an exit code 2.

Can you make a PR if you don't mind? Otherwise, it falls under I will get to it someday.

  • func (c *CmdLint) run() ([]report.Failure, error) {
    var allFailures []report.Failure
    for _, f := range c.protoFiles {
    failures, err := c.runOneFile(f)
    if err != nil {
    return nil, err
    }
    allFailures = append(allFailures, failures...)
    }
    return allFailures, nil
    }
    is a related part, in particular c.protoFiles.

@jd1378
Copy link
Contributor Author

jd1378 commented Jul 25, 2020

I made a pull request, see if it looks good

@yoheimuta
Copy link
Owner

isn't it better to exit with 0 when there's nothing to lint ?

I understand your point. Can you show me some linters that behave so?
For example, eslint that has immense popularity exits with code 0 when it found no files.

Sorry, I misunderstood the current spec opposite 😓
(But #134 looked good to me regardless of the interpretation, as the first step.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants