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

Remove BOM_SKIP_ARTIFACT_IDS #2338

Merged
merged 11 commits into from
May 15, 2023
Merged

Remove BOM_SKIP_ARTIFACT_IDS #2338

merged 11 commits into from
May 15, 2023

Conversation

suztomo
Copy link
Contributor

@suztomo suztomo commented May 11, 2023 •
edited
Loading

The BOM_SKIP_ARTIFACT_IDS has been hiding the google-cloud-logging-logback artifact from appearing in the release note. https://github.com/googleapis/java-cloud-bom/releases/tag/v26.14.0

@suztomo suztomo added the kokoro:run force run kokoro label May 11, 2023
@cloud-tools-for-java-team cloud-tools-for-java-team removed the kokoro:run force run kokoro label May 11, 2023
@suztomo
Copy link
Contributor Author

suztomo commented May 11, 2023

kokoro-windows failed:

C:\tmpfs\src\github\cloud-opensource-java>call mvnw.cmd -V -B -ntp clean install javadoc:jar 

Error: JAVA_HOME is set to an invalid directory. 
JAVA_HOME = "c:\program files\java\jdk1.8.0_152" 
Please set the JAVA_HOME variable in your environment to match the 
location of your Java installation.

@suztomo suztomo added the kokoro:run force run kokoro label May 11, 2023
@cloud-tools-for-java-team cloud-tools-for-java-team removed the kokoro:run force run kokoro label May 11, 2023
@suztomo suztomo added the kokoro:run force run kokoro label May 11, 2023
@cloud-tools-for-java-team cloud-tools-for-java-team removed the kokoro:run force run kokoro label May 11, 2023
@suztomo suztomo added the kokoro:run force run kokoro label May 12, 2023
suztomo
suztomo commented May 12, 2023
View reviewed changes
dependencies/src/main/java/com/google/cloud/tools/opensource/dependencies/Bom.java
Comment on lines -155 to -158
// TODO remove this hack once we get these out of google-cloud-java's BOM
if (BOM_SKIP_ARTIFACT_IDS.contains(artifact.getArtifactId())) {
return true;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the main change.

@cloud-tools-for-java-team cloud-tools-for-java-team removed the kokoro:run force run kokoro label May 12, 2023
@suztomo
Copy link
Contributor Author

suztomo commented May 13, 2023

Kokoro-macos passed

@suztomo
Copy link
Contributor Author

suztomo commented May 15, 2023

"Build with Sonar" is working in the master branch.
https://github.com/GoogleCloudPlatform/cloud-opensource-java/actions/runs/4978807848/jobs/8909590064
Why is it not working in this pull request?

suztomo
suztomo commented May 15, 2023
View reviewed changes
kokoro/continuous.bat
@@ -1,6 +1,6 @@
@echo on

set JAVA_HOME=c:\program files\java\jdk1.8.0_152
set JAVA_HOME=c:\program files\java\jdk1.8.0_211
Copy link
Contributor Author

Choose a reason for hiding this comment

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

_152 was not owrking any more.

dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/ExclusionFilesTest.java
Comment on lines -343 to +351
String actual = new String(Files.readAllBytes(output));
String actual = new String(Files.readAllBytes(output)).replaceAll("\\R", "\n");

String expected =
new String(
Files.readAllBytes(
absolutePathOfResource(
"exclusion-sample-rules/expected-exclusion-output-file.xml")),
StandardCharsets.UTF_8);
Files.readAllBytes(
absolutePathOfResource(
"exclusion-sample-rules/expected-exclusion-output-file.xml")),
StandardCharsets.UTF_8)
.replaceAll("\\R", "\n");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Memo: In the new Windows environment, which changed jdk1.8.0_152 to jdk1.8.0_211, the return character was different. This change ignores the difference.

dashboard/src/test/java/com/google/cloud/tools/opensource/dashboard/DashboardTest.java
Comment on lines -472 to +473
assertThat(dependencyTreeParagraph).hasSize(38391);
assertThat(dependencyTreeParagraph).hasSize(39649);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The assertion count changes because now the BOM includes a bit more artifacts.

@suztomo
Copy link
Contributor Author

suztomo commented May 15, 2023

SonarCloud Analysis / Build with Sonar (pull_request)

Maybe it's not working on pull requests. If this fails on master branch after merge, I'll troubleshoot.

@suztomo suztomo merged commit d9c56b0 into GoogleCloudPlatform:master May 15, 2023
@suztomo suztomo deleted the fix_artifact_exclusions branch May 15, 2023 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoeWang1127 JoeWang1127 JoeWang1127 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@suztomo @JoeWang1127 @cloud-tools-for-java-team