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

Misc leftover changes discovered while updating protobuff version #6736

Closed
wants to merge 1 commit into from

Conversation

lbergelson
Copy link
Member

Moving us off of of the weird beta protobuf version to the newest 3.x release.

@lbergelson lbergelson requested a review from droazen August 3, 2020 16:22
@lbergelson lbergelson assigned lbergelson and droazen and unassigned lbergelson Aug 3, 2020
@lbergelson
Copy link
Member Author

@droazen This is good now I think.

.travis.yml Outdated
@@ -21,6 +21,7 @@ env:
- SCALA_VERSION=2.11 TEST_TYPE=wdlGen
global:
#gradle needs this
- NO_GCE_CHECK=true
Copy link
Member Author

Choose a reason for hiding this comment

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

This is irrelevant but we might as well put it in. Hmn, I should move it above the comment though...

@@ -302,7 +302,7 @@ public static void assertGenotypesAreEqual(final Genotype actual, final Genotype

public static void assertGenotypesAreEqual(final Genotype actual, final Genotype expected, final List<String> extendedAttributesToIgnore) {
Assert.assertEquals(actual.getSampleName(), expected.getSampleName(), "Genotype names");
Assert.assertTrue(CollectionUtils.isEqualCollection(actual.getAlleles(), expected.getAlleles()), "Genotype alleles");
Assert.assertTrue(CollectionUtils.isEqualCollection(actual.getAlleles(), expected.getAlleles()), "Genotype alleles: " + actual.getAlleles() + " != " + expected.getAlleles());
Copy link
Member Author

Choose a reason for hiding this comment

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

This change came up while testing things but I figured it's useful so I'd keep it.

@droazen
Copy link
Contributor

droazen commented Sep 3, 2020

Most of these dependency changes are reflected in #6759. We should let that PR go in first, then rebase this one.

@droazen
Copy link
Contributor

droazen commented Oct 2, 2020

@lbergelson Now that #6759 is merged, can you rebase this branch onto master to reconcile it with the extensive dependency changes in that PR?

@droazen droazen assigned lbergelson and unassigned droazen Oct 2, 2020
@lbergelson lbergelson force-pushed the lb_move_to_sane_protobuff branch from c28efd2 to 8a8fea7 Compare October 29, 2020 19:37
@lbergelson lbergelson force-pushed the lb_move_to_sane_protobuff branch from 8a8fea7 to 1694572 Compare May 12, 2021 19:48
@lbergelson lbergelson changed the title Move to a sane version of protobuf Misc leftover changes discovered while updating protobuff version May 12, 2021
@lbergelson
Copy link
Member Author

@droazen there are 3 minor changes here that were left over after the other branch got merged. The only controversial one might be unforcing the protobuff version, but I think it makes sense to just let it float at whatever the other dependencies require since that's what everything else does.

@lbergelson
Copy link
Member Author

Closing this. Moved the 1 relevant change to #8725

@lbergelson lbergelson closed this Mar 8, 2024
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.

2 participants