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

Add project_id to Android sign script #967

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Add project_id to Android sign script #967

wants to merge 12 commits into from

Conversation

enahum
Copy link
Contributor

@enahum enahum commented Nov 18, 2021

Summary

Added project_id to be replaced in the strings resources as is needed by Firebase Cloud Messaging to be able to initialize and retrieve the device token to receive notifications.

@enahum enahum added the 1: Dev Review Requires review by a core commiter label Nov 18, 2021
@enahum enahum requested a review from sadohert November 18, 2021 11:08
@github-actions
Copy link

Newest code from enahum has been published to preview environment for Git SHA 88cbc6b

1 similar comment
@github-actions
Copy link

Newest code from enahum has been published to preview environment for Git SHA 88cbc6b

@sadohert
Copy link
Contributor

OOOO... nice fix for the ZIPALIGNER APKSIGNER issue too!

I'll download this, sign an unsigned bundle, and test... I've done it once, but will use the published version to QA this.

@sadohert
Copy link
Contributor

Okay, test checks out.

Only question I have is how do we handle the fact that the google-services.json file I get from Firebase is missing the firebase_url property...

I can't see a way for firebase to provide this? Is it possible we could just form the firebase_database_url property we inject into string.xml ourselves, so customers dont' have to add it? Does our app code care about this property, or is it in the underlying SDKs/etc.?

@enahum

@enahum
Copy link
Contributor Author

enahum commented Nov 18, 2021

That is probably a good idea

@sadohert
Copy link
Contributor

I will take a stab at this. I'm still puzzled about why we don't get the firebase_url property in google-services.json. I mentioned in an SO comment.

Although, now that I think about it, if the full end to end build process still works then maybe the Google Services Maven plugin has already been updated to handle this 🤷🏼‍♂️?

@github-actions
Copy link

Newest code from sadohert has been published to preview environment for Git SHA b53ed52

PROJECT_NUMBER=$(echo "$GOOGLE_SERVICES" | jq -r .project_info.project_number)
FIREBASE_URL=$(echo "$GOOGLE_SERVICES" | jq -r .project_info.firebase_url)
FIREBASE_URL="https://$PROJECT_ID.firebaseio.com"
echo "Setting FIREBASE_URL to: $FIREBASE_URL"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sadohert maybe remove this echo ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I left the echo in as a bit of a breadcrumb in case the logic picked something up incorrectly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

alright

@github-actions
Copy link

Newest code from enahum has been published to preview environment for Git SHA 93322ec

@github-actions
Copy link

Newest code from hanzei has been published to preview environment for Git SHA 7e9c8ef

@github-actions
Copy link

github-actions bot commented Jun 7, 2022

Newest code from cwarnermm has been published to preview environment for Git SHA aa157f3

@cwarnermm
Copy link
Member

@sadohert - Friendly ping on this PR. Please note that a second reviewer is required for this repository.

@sadohert
Copy link
Contributor

sadohert commented Jun 8, 2022

Hey - Apologies.. I didnt' know I still had to do something here.

@enahum how relevant is all this with Gekidou?

@github-actions
Copy link

Newest code from cwarnermm has been published to preview environment for Git SHA 0bd8548

@github-actions
Copy link

Newest code from cwarnermm has been published to preview environment for Git SHA 85b8cad

@cwarnermm cwarnermm requested a review from neflyte October 3, 2022 14:29
@github-actions
Copy link

github-actions bot commented Oct 3, 2022

Newest code from cwarnermm has been published to preview environment for Git SHA 54831c5

@cwarnermm cwarnermm removed the request for review from neflyte October 4, 2022 12:20
@github-actions
Copy link

github-actions bot commented Oct 4, 2022

Newest code from cwarnermm has been published to preview environment for Git SHA 813ef90

@cwarnermm
Copy link
Member

@sadohert - Friendly reminder to review this PR. Thanks!

@github-actions
Copy link

Newest code from cwarnermm has been published to preview environment for Git SHA f14c8b0

@cwarnermm
Copy link
Member

@enahum @sadohert - Do we want to keep this PR open?

@sadohert
Copy link
Contributor

@cwarnermm I'm not likely to get to this for a while, but I also don't want to lose the change if it comes up again with another customer (hard to know when this will rear its head).... is there a place this PR can sit that isn't in the way, but isn't gone for good?

@cwarnermm
Copy link
Member

Happy to leave it right here and open, @sadohert.

@github-actions
Copy link

Newest code from cwarnermm has been published to preview environment for Git SHA 82f017a

@hanzei hanzei added Lifecycle/frozen and removed 1: Dev Review Requires review by a core commiter labels Oct 10, 2023
Copy link

github-actions bot commented Mar 4, 2024

Newest code from hanzei has been published to preview environment for Git SHA 1032b94

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants