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

Version code #941

Merged
merged 3 commits into from
Jul 21, 2023
Merged

Version code #941

merged 3 commits into from
Jul 21, 2023

Conversation

bmarty
Copy link
Member

@bmarty bmarty commented Jul 21, 2023

Fix downgrade issue due to version code recent change (in #932).
Also:

  • Firebase will get the app bundle instead of the APK.
  • Maestro will get the x86_64 APK instread of the universal one.

@bmarty bmarty requested a review from a team as a code owner July 21, 2023 09:33
@bmarty bmarty requested review from ganfra and removed request for a team July 21, 2023 09:33
@github-actions
Copy link
Contributor

📱 Scan the QR code below to install the build (arm64 only) for this PR.
QR code
If you can't scan the QR code you can install the build via this link:

@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (6dcdf7d) 56.63% compared to head (75aaa92) 56.63%.

Additional details and impacted files 
@@           Coverage Diff            @@
##           develop     #941   +/-   ##
========================================
  Coverage    56.63%   56.63%           
========================================
  Files          968      968           
  Lines        24592    24592           
  Branches      4987     4987           
========================================
  Hits         13927    13927           
  Misses        8456     8456           
  Partials      2209     2209

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sonarcloud
Copy link

sonarcloud bot commented Jul 21, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@bmarty bmarty removed the request for review from ganfra July 21, 2023 09:54
csmith
csmith approved these changes Jul 21, 2023
View reviewed changes
Copy link
Contributor

@csmith csmith left a comment

Choose a reason for hiding this comment

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

LGTM, couple of suggestions but they can be done later if necessary :)

.github/workflows/nightly.yml
@@ -44,7 +44,9 @@ jobs:
FIREBASE_TOKEN: ${{ secrets.ELEMENT_ANDROID_NIGHTLY_FIREBASE_TOKEN }}
- name: Additionally upload Nightly APK to browserstack for testing
continue-on-error: true # don't block anything by this upload failing (for now)
run: curl -u "$BROWSERSTACK_USERNAME:$BROWSERSTACK_PASSWORD" -X POST "https://api-cloud.browserstack.com/app-automate/upload" -F "file=@app/build/outputs/apk/nightly/app-universal-nightly.apk" -F "custom_id=element-x-android-nightly"
run: |
./gradlew assembleNightly $CI_GRADLE_ARG_PROPERTIES
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I guess we'll need the ELEMENT_ANDROID_MAPTILER_* env vars in this task if we're now building here

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes, good point, added.

plugins/src/main/kotlin/Versions.kt
* Version when running the current debug build
* -------10_200
*
* So adding 4_000_000 to the current version Code computed here should be fine, we will have:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe mention the "multiply by 10 and add a number for the abi" logic somewhere here? It just looks like the maths is wrong at the minute 😆

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you're right! Done.

@bmarty bmarty merged commit 301cb82 into develop Jul 21, 2023
@bmarty bmarty deleted the feature/bma/versionCode branch July 21, 2023 10:18
@bmarty
Copy link
Member Author

bmarty commented Jul 21, 2023 •
edited
Loading

Oups, I have merged without pushing my latest commits, I will push them on develop:

@bmarty
Copy link
Member Author

bmarty commented Jul 21, 2023

There is an issue with Firebase Distribution that I am investigating:

 > Task :app:appDistributionUploadNightly FAILED
Using AAB path specified by the artifactPath parameter in your app's build.gradle: /home/runner/work/element-x-android/element-x-android/app/build/outputs/bundle/nightly/app-nightly.aab.
Uploading AAB to Firebase App Distribution...
Using credentials token specified by environment variable FIREBASE_TOKEN

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':app:appDistributionUploadNightly'.
> App Distribution failed to process the AAB: This project is not linked to a Google Play account.

Good news is that Mastro is happy with the x86_64 APK.

@bmarty
Copy link
Member Author

bmarty commented Jul 21, 2023

When doing it manually, I've got:

image

@bmarty
Copy link
Member Author

bmarty commented Jul 21, 2023 •
edited
Loading

App on Firebase is now linked to GooglePlay (I had to create a dummy app on GooglePlay to do that), to be able to release nightly build as AAB

image

But it cannot work since the dummy app on Google Play is not published. Will try to publish internally only.

image

In the meantime, I will roll back the change on the nightly build, let's push universal APK (to unblock next nightlies).

bmarty added a commit that referenced this pull request Jul 21, 2023
@bmarty
Revert change from #941, upload APK to Firebase App Distribution for …
f0874fd 
…now.

We need a matching and released PlayStore application to be able to upload an AAB, and we do not have that for now.
@bmarty bmarty mentioned this pull request Jul 21, 2023
Merged
@bmarty
Copy link
Member Author

bmarty commented Jul 21, 2023

So reverting this change in #945 to unblock the nightly flow.

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

@csmith csmith csmith 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.
2 participants
@bmarty @csmith