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

Update FirebaseRemoteConfig.java #11

Merged
merged 3 commits into from
Sep 27, 2023
Merged

Update FirebaseRemoteConfig.java #11

merged 3 commits into from
Sep 27, 2023

Conversation

teknepal
Copy link

@teknepal teknepal commented Jun 9, 2022

No description provided.

@gugahoi
Copy link

gugahoi commented Jun 9, 2022

Thanks for this change. Just so I am clear: is the current implementation incorrect? Does that method not exist?

@teknepal
Copy link
Author

teknepal commented Jun 9, 2022

@gugahoi yes, current implementation for android is incorrect based on this:
https://firebase.google.com/docs/remote-config/get-started?platform=android or https://github.com/firebase/snippets-android/blob/77870d2c0f6654b5a7582d2d647495c49b04afd5/config/app/src/main/java/com/google/firebase/quickstart/config/java/MainActivity.java#L32-L35

@uniqbit-julian uniqbit-julian mentioned this pull request Nov 22, 2022
Open
@muhrifqii
Copy link

will this be merged? @gugahoi

@anonimitoraf
Copy link

@gugahoi looks like the method exists

image

However, I don't think that it's the right method to use. We want to use setMinimumFetchIntervalInSeconds (as @teknepal has in their PR)

image

anonimitoraf
anonimitoraf reviewed Sep 26, 2023
View reviewed changes
Copy link

@anonimitoraf anonimitoraf left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @teknepal ! Sorry for the delay, do you mind quickly resolving the merge conflict?

@teknepal
Copy link
Author

@anonimitoraf done, please merge and publish to npm.

gugahoi
gugahoi approved these changes Sep 27, 2023
View reviewed changes
Copy link

@gugahoi gugahoi left a comment

Choose a reason for hiding this comment

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

Thank you for catching this @teknepal!

@gugahoi gugahoi merged commit 609132e into joinflux:master Sep 27, 2023
@gugahoi
Copy link

gugahoi commented Sep 27, 2023

This is now in the 1.0.1 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anonimitoraf anonimitoraf anonimitoraf left review comments

@gugahoi gugahoi gugahoi 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.
5 participants
@teknepal @gugahoi @muhrifqii @anonimitoraf @martingarcia95