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

feat!: migrate firebase_remote_config nullsafety #5397

Closed
wants to merge 24 commits into from
Closed

feat!: migrate firebase_remote_config nullsafety #5397

wants to merge 24 commits into from

Conversation

daniloadorno
Copy link
Contributor

@daniloadorno daniloadorno commented Mar 19, 2021

No description provided.

@google-cla
Copy link

google-cla bot commented Mar 19, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Mar 19, 2021
@google-cla google-cla bot added cla: yes and removed cla: no labels Mar 19, 2021
@rrousselGit
Copy link
Contributor

@rrousselGit rrousselGit changed the title migrate firebase_remote_config and firebase_analytics to nullsafety chore: migrate firebase_remote_config and firebase_analytics to nullsafety Mar 22, 2021
@rrousselGit rrousselGit changed the title chore: migrate firebase_remote_config and firebase_analytics to nullsafety BREAKING CHANGE: migrate firebase_remote_config and firebase_analytics to nullsafety Mar 22, 2021
@rrousselGit rrousselGit changed the title BREAKING CHANGE: migrate firebase_remote_config and firebase_analytics to nullsafety feat!: migrate firebase_remote_config and firebase_analytics to nullsafety Mar 22, 2021
@Salakar
Copy link
Member

Salakar commented Mar 22, 2021

I think the Analytics portion of this PR may be a duplicate of the work being done on #5341 - can we perhaps remove it from this PR and focus on just Remote Config here? Thoughts @rrousselGit?

@IchordeDionysos
Copy link
Contributor

@Salakar @rrousselGit also in this PR are the same issues that @rrousselGit mentioned that I already fixed in my PR :)

@Salakar
Copy link
Member

Salakar commented Mar 22, 2021

@Salakar @rrousselGit also in this PR are the same issues that @rrousselGit mentioned that I already fixed in my PR :)
@IchordeDionysos

Let's focus on Remote config for this PR - @daniloadorno would you mind removing the analytics portions of this pull request - thanks.

@b-jan
Copy link

b-jan commented Mar 24, 2021

@daniloadorno I saw you worked on the migration. Is it ready ?

@daniloadorno
Copy link
Contributor Author

@daniloadorno I saw you worked on the migration. Is it ready ?

yes

@rrousselGit rrousselGit changed the title feat!: migrate firebase_remote_config and firebase_analytics to nullsafety feat!: migrate firebase_remote_config nullsafety Mar 25, 2021
@rrousselGit
Copy link
Contributor

Could you format and fix the test step?

@daniloadorno
Copy link
Contributor Author

Could you format and fix the test step?

done

await remoteConfig.fetchAndActivate();
expect(remoteConfig.lastFetchStatus, RemoteConfigFetchStatus.success);
//await remoteConfig.fetchAndActivate();
/*expect(remoteConfig.lastFetchStatus, RemoteConfigFetchStatus.success);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these tests commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they refer to the old implementation without nullsafety and have errors in their execution

@google-cla
Copy link

google-cla bot commented Apr 7, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no and removed cla: yes labels Apr 7, 2021
@google-cla
Copy link

google-cla bot commented Apr 7, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-oss-bot google-oss-bot added Needs Attention This issue needs maintainer attention. and removed blocked: customer-response Waiting for customer response, e.g. more information was requested. labels Apr 7, 2021
@google-cla
Copy link

google-cla bot commented Apr 7, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

1 similar comment
@google-cla
Copy link

google-cla bot commented Apr 7, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@Zeswen
Copy link

Zeswen commented Apr 7, 2021

@rrousselGit I believe there's a problem with @daniloadorno's commits. It seems like he commited changes with an alter account not related to GitHub and the Google checks are not passing.

Any thoughts?

@daniloadorno
Copy link
Contributor Author

@rrousselGit #5686

@firebase firebase locked and limited conversation to collaborators May 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: no Needs Attention This issue needs maintainer attention. plugin: remote_config
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants