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

Update deploy.yml #1683

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update deploy.yml #1683

wants to merge 1 commit into from

Conversation

y4ssi
Copy link
Contributor

@y4ssi y4ssi commented Nov 14, 2024

Fixing CI

Copy link
Collaborator

@HonzaR HonzaR left a comment

Choose a reason for hiding this comment

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

Looks good to me. I just block this until we finish the current Zashi release and re-review it after it.

Comment on lines +112 to +113
echo $FIREBASE_DEBUG_JSON_BASE64 | base64 --decode > app/src/debug/google-services.json
echo $FIREBASE_RELEASE_JSON_BASE64 | base64 --decode > app/src/release/google-services.json
Copy link

Choose a reason for hiding this comment

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

Suggested change
echo $FIREBASE_DEBUG_JSON_BASE64 | base64 --decode > app/src/debug/google-services.json
echo $FIREBASE_RELEASE_JSON_BASE64 | base64 --decode > app/src/release/google-services.json
echo "$FIREBASE_DEBUG_JSON_BASE64" | base64 --decode > app/src/debug/google-services.json
echo "$FIREBASE_RELEASE_JSON_BASE64" | base64 --decode > app/src/release/google-services.json

Copy link

Choose a reason for hiding this comment

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

Also double-quote $GITHUB_OUTPUT on line 72.

- name: Export Google Services JSON
env:
FIREBASE_DEBUG_JSON_BASE64: ${{ secrets.FIREBASE_DEBUG_JSON_BASE64 }}
FIREBASE_RELEASE_JSON_BASE64: ${{ secrets.FIREBASE_RELEASE_JSON_BASE64 }}
if: "${{ env.FIREBASE_DEBUG_JSON_BASE64 != '' && env.FIREBASE_RELEASE_JSON_BASE64 != '' }}"
shell: bash
if: ${{ env.FIREBASE_DEBUG_JSON_BASE64 != '' && env.FIREBASE_RELEASE_JSON_BASE64 != '' }}
Copy link

@daira daira Nov 20, 2024

Choose a reason for hiding this comment

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

If the syntax with double quotes around a GitHub Actions expression causes a problem, note that it also occurs on lines 67-71. (You can omit the ${{ and }} when used with if:, btw.)

run: |
echo ${SIGNING_KEYSTORE_BASE_64} | base64 --decode > ${SIGNING_KEY_PATH}
echo $SIGNING_KEYSTORE_BASE_64 | base64 --decode > $SIGNING_KEY_PATH
Copy link

Choose a reason for hiding this comment

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

Suggested change
echo $SIGNING_KEYSTORE_BASE_64 | base64 --decode > $SIGNING_KEY_PATH
echo "$SIGNING_KEYSTORE_BASE_64" | base64 --decode > "$SIGNING_KEY_PATH"

Comment on lines +149 to +151
mkdir $ARTIFACTS_DIR_PATH
zip -r $BINARIES_ZIP_PATH . -i app/build/outputs/apk/*/*.apk app/build/outputs/apk_from_bundle/*/*.apk app/build/outputs/bundle/*/*.aab
zip -r $MAPPINGS_ZIP_PATH . -i app/build/outputs/mapping/*/mapping.txt
Copy link

Choose a reason for hiding this comment

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

Suggested change
mkdir $ARTIFACTS_DIR_PATH
zip -r $BINARIES_ZIP_PATH . -i app/build/outputs/apk/*/*.apk app/build/outputs/apk_from_bundle/*/*.apk app/build/outputs/bundle/*/*.aab
zip -r $MAPPINGS_ZIP_PATH . -i app/build/outputs/mapping/*/mapping.txt
mkdir "$ARTIFACTS_DIR_PATH"
zip -r "$BINARIES_ZIP_PATH" . -i app/build/outputs/apk/*/*.apk app/build/outputs/apk_from_bundle/*/*.apk app/build/outputs/bundle/*/*.aab
zip -r "$MAPPINGS_ZIP_PATH" . -i app/build/outputs/mapping/*/mapping.txt

Copy link

@daira daira left a comment

Choose a reason for hiding this comment

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

utACK with suggestions.

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.

3 participants