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

Exclude auto-generated protobuf files #81

Closed
wants to merge 1 commit into from

Conversation

upgle
Copy link
Member

@upgle upgle commented Apr 6, 2021

Description

It excludes files auto-generated by the Scala Plugin for the Protocol Buffer Compiler.
I couldn't find a way to add a license header for the auto-generated file.

Those excluded protobuf files will be used for the new scheduler contribution (GRPC).

Related issue

apache/openwhisk#5070

Copy link
Member

@style95 style95 left a comment

Choose a reason for hiding this comment

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

LGTM

@dgrove-oss
Copy link
Member

dgrove-oss commented Apr 6, 2021

Do the generated files need to be checked into git? Or are they artifacts produced by doing a build? If produced by the build, they should not need to be excluded from scan code. We can run the scan code on the fresh clone (before starting build).

@upgle
Copy link
Member Author

upgle commented Apr 6, 2021

@dgrove-oss
Thank you for your comment. Let me check if the code can be scanned on the fresh clone.

@dgrove-oss
Copy link
Member

I took a look at .travis.yml. We intentionally compile the source tree "early" in the install stage from setup.sh while we defer scancode.sh to the runUnitTests.sh job. So, our choices are either exclude the generated sources as this PR does or to move the invocation of scan.sh into the install stage (which would cause it to run on all jobs in the matrix in the install stage). It would be more consistent with the other openwhisk repos to move the scan early, but either approach could be made to work.

@upgle
Copy link
Member Author

upgle commented Apr 6, 2021

Thank you for taking a closer look. I've moved the invocation of scan.sh into the install stage in this commit. apache/openwhisk@4d9c7c6

It runs on all jobs in matrices, but it doesn't take much time to do a code scanning. And I think this approach would be more suitable for handling various cases for auto-generated files.

@upgle
Copy link
Member Author

upgle commented Apr 6, 2021

Close this issue by handling it in another way.

@upgle upgle closed this Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants