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

Fix the compatibility workflows #2899

Merged
merged 2 commits into from
Nov 23, 2021

Conversation

pradeepbn
Copy link
Contributor

@pradeepbn pradeepbn commented Nov 17, 2021

Descriptions of the changes in this PR:

  • Fix the compatibility workflows for java11 and java8
  • Fixes the dependencies in the tests

Motivation

Compatibility checks workflow wasn't working because there the github workflow.yaml had syntax issues. Also, since it was enabled 1st time, there were build issues with the tests.

@pradeepbn pradeepbn marked this pull request as draft November 17, 2021 04:51
@pradeepbn pradeepbn force-pushed the fixCompatibilityWorkflow branch from e38af20 to 883f770 Compare November 17, 2021 18:22
@pradeepbn pradeepbn marked this pull request as ready for review November 19, 2021 06:43
@pkumar-singh
Copy link
Member

rerun failure checks

@pradeepbn pradeepbn force-pushed the fixCompatibilityWorkflow branch from 125aaed to c4b67d6 Compare November 19, 2021 23:31
Copy link
Contributor

@Vanlightly Vanlightly left a comment

Choose a reason for hiding this comment

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

LGTM

@Vanlightly
Copy link
Contributor

@eolivelli could PTAL? If all is good would be good to merge.

Copy link
Contributor

@nicoloboschi nicoloboschi left a comment

Choose a reason for hiding this comment

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

Great work, I think we should not change the current tests about shaded DL classpath

@@ -59,7 +59,7 @@ public void testProtobufIsShaded() throws Exception {

@Test
public void testProtobufShadedPath() throws Exception {
Class.forName("dlshade.com.google.protobuf.Message");
Class.forName("org.apache.bookkeeper.shaded.com.google.protobuf.Message");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this? I think there is a duplicate rule for com.google in the build.gradle

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch- thanks. Its fixed now.

@eolivelli
Copy link
Contributor

Tests didn't pass.
I have restarted them

@Vanlightly Vanlightly merged commit b570107 into apache:master Nov 23, 2021
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
* Fix workflow yaml for compatibility tests

* remove duplicate of com.google relocate in build.gradle
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.

5 participants