-
Notifications
You must be signed in to change notification settings - Fork 26
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
false positives in protobuf generated code #17
Comments
If you're interested in detecting generated files, please be aware of golang/go#13560 and https://godoc.org/github.com/shurcooL/go/analysis#IsFileGenerated. I'm not sure if linters usually try to detect or skip generated files. Perhaps it's a valid option to offer (maybe even turn on by default). |
Thanks for the issue reference. I figured I couldn't have been the first person to run into this. Yeah, I have mixed feelings skipping over machine generated code. The protobuf case at least seems simple enough, since all of the code is machine generated. Other cases (e.g., the Go compiler's old yacc-based parser) seem trickier, since they mix hand-written and machine-generated code. |
@tamird Do you have any thoughts here for cockroachdb? I'm fine skipping .pb.go files and already wrote the code for it, but want to make sure there's user interest before committing. (I'm not using protobufs and unconvert together in any projects currently.) |
I don't feel that strongly about this - we're easily ignoring those files with a simple grep. Perhaps a flag that ignored files based on regex would be useful, but again not important IMO. |
Okay. If filtering the results with grep is satisfactory, I'll leave it as is for now. |
After fixing #16, all of the remaining issues in github.com/cockroachdb/cockroach/... are now in machine generate protobuf files.
I'm thinking unconvert should just ignore files ending with ".pb.go", but I'm open to suggestions for alternative heuristics. E.g., are there any strings like "DO NOT EDIT" that are conventionally recognized by other linter tools?
The text was updated successfully, but these errors were encountered: