Skip to content

Conversation

lahirumaramba
Copy link
Member

  • Add getTemplateAtVersion(), publishTemplate(), validateTemplate(), and forcePublishTemplate() operations.
  • Add unit tests for new operations.
  • Add new unit test for timestamps with up to nanoseconds precision.

Related to: #446

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Looks pretty good. Just a couple of suggestion on improving the implementation and cleaning up the tests.

}

@Test(expected = IllegalStateException.class)
public void testGetTemplateAtVersionWithInvalidEtags() throws FirebaseRemoteConfigException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Which line is supposed to throw in this test case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! It should throw on client.getTemplateAtVersion("24");. Converted into to two separate tests.

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM with a suggestion.

@@ -0,0 +1,33 @@
package com.google.firebase.remoteconfig.internal;
Copy link
Contributor

Choose a reason for hiding this comment

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

This can probably live in the parent package as a package-protected class.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call! Moved to the parent class and made package protected, including the ctor. Thanks!

@lahirumaramba lahirumaramba merged commit 1a3f036 into remote-config Nov 24, 2020
@lahirumaramba lahirumaramba deleted the lm-rc-publish branch November 24, 2020 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants