Skip to content

Update static analysis image to include java 8 and 11 runtimes. #3769

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
Merged
merged 2 commits into from
Sep 22, 2020
Merged

Update static analysis image to include java 8 and 11 runtimes. #3769

merged 2 commits into from
Sep 22, 2020

Conversation

dzlier-gcp
Copy link
Member

@dzlier-gcp dzlier-gcp commented Sep 21, 2020 •
edited
Loading

Fixes #issue

It's a good idea to open an issue first for discussion.

  • I have followed Sample Format Guide
  • Static Analysis: mvn -P lint clean compile pmd:cpd-check spotbugs:check advisory only

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 21, 2020
@dzlier-gcp dzlier-gcp added the kokoro:run Add this label to force Kokoro to re-run the tests. label Sep 21, 2020
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Sep 21, 2020
@dzlier-gcp dzlier-gcp marked this pull request as ready for review September 21, 2020 22:39
@dzlier-gcp dzlier-gcp requested a review from a team September 21, 2020 22:39
kurtisvg
kurtisvg suggested changes Sep 21, 2020
View reviewed changes
lesv
lesv reviewed Sep 22, 2020
View reviewed changes
Copy link
Contributor

@lesv lesv left a comment

Choose a reason for hiding this comment

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

@dzlier-gcp This should have been discussed at the Java meeting today.

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Sep 22, 2020
@dzlier-gcp
Copy link
Member Author

@dzlier-gcp This should have been discussed at the Java meeting today.

I didn't think I'd be working in this repo, I just noticed that static analysis has been failing while I was resolving a Healthcare Datasets bug.

Should I close this for now for further discussion, or just remove the unused env var?

@kurtisvg
Copy link
Contributor

This is only controlling which JVM version the static analysis check is being run with. It seems fine to run it on Java 11 over Java 8, since some samples only support Java 11 or higher. This seems okay to merge unless I'm missing smething.

It's likely still going to fail for a majority of the samples, as we discussed in the Java maintenance meeting the checks are overly aggressive (which Les is looking into).

@dzlier-gcp
Copy link
Member Author

This is only controlling which JVM version the static analysis check is being run with. It seems fine to run it on Java 11 over Java 8, since some samples only support Java 11 or higher. This seems okay to merge unless I'm missing smething.

It's likely still going to fail for a majority of the samples, as we discussed in the Java maintenance meeting the checks are overly aggressive (which Les is looking into).

All right, I need a new approval after removing that env var though.

@dzlier-gcp
Copy link
Member Author

Oh nvm

@dzlier-gcp dzlier-gcp merged commit 497464f into GoogleCloudPlatform:master Sep 22, 2020
@lesv lesv mentioned this pull request Sep 22, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lesv lesv lesv left review comments

@kurtisvg kurtisvg kurtisvg approved these changes

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dzlier-gcp @kurtisvg @lesv @kokoro-team