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

Remove usage of Buildkite. #7583

Merged
merged 9 commits into from
Nov 24, 2022
Merged

Remove usage of Buildkite. #7583

merged 9 commits into from
Nov 24, 2022

Conversation

bmarty
Copy link
Member

@bmarty bmarty commented Nov 14, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Remove usage of Buildkite.
Build number is just removed.
Related script will need to be updated separately.

Motivation and context

Closes #7493

Release script will have to be updated for the next release, but it's not part of the project file.

TODO:

Screenshots / GIFs

Tests

  • NA

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@bmarty bmarty requested review from a team and mnaturel and removed request for a team November 14, 2022 14:27
@ElementBot
Copy link

ElementBot commented Nov 14, 2022

Warnings
⚠️

vector/src/main/AndroidManifest.xml#L201 - Consider splitting data tag into multiple tags with individual attributes to avoid confusion

⚠️

vector/src/main/AndroidManifest.xml#L201 - Consider splitting data tag into multiple tags with individual attributes to avoid confusion

⚠️

vector/src/main/AndroidManifest.xml#L204 - Consider splitting data tag into multiple tags with individual attributes to avoid confusion

⚠️

vector/src/main/AndroidManifest.xml#L204 - Consider splitting data tag into multiple tags with individual attributes to avoid confusion

⚠️

vector/src/main/AndroidManifest.xml#L258 - Expecting android:screenOrientation="unspecified" or "fullSensor" for this activity so the user can use the application in any orientation and provide a great experience on Chrome OS devices

⚠️

vector/src/main/AndroidManifest.xml#L258 - Expecting android:screenOrientation="unspecified" or "fullSensor" for this activity so the user can use the application in any orientation and provide a great experience on Chrome OS devices

⚠️

vector/src/main/AndroidManifest.xml#L267 - Attribute supportsPictureInPicture is only used in API level 24 and higher (current min is 21)

⚠️

vector/src/main/AndroidManifest.xml#L267 - Attribute supportsPictureInPicture is only used in API level 24 and higher (current min is 21)

⚠️

vector/src/main/AndroidManifest.xml#L274 - Attribute supportsPictureInPicture is only used in API level 24 and higher (current min is 21)

⚠️

vector/src/main/AndroidManifest.xml#L274 - Attribute supportsPictureInPicture is only used in API level 24 and higher (current min is 21)

⚠️

vector/src/main/AndroidManifest.xml#L280 - Attribute supportsPictureInPicture is only used in API level 24 and higher (current min is 21)

⚠️

vector/src/main/AndroidManifest.xml#L280 - Attribute supportsPictureInPicture is only used in API level 24 and higher (current min is 21)

⚠️

vector/src/main/AndroidManifest.xml#L400 - Exported receiver does not require permission

⚠️

vector/src/main/AndroidManifest.xml#L400 - Exported receiver does not require permission

Generated by 🚫 dangerJS against 492e842

- [ ] Wait for [Buildkite](https://buildkite.com/matrix-dot-org/element-android/builds?branch=main) to build the `main` branch.
- [ ] Run the script `~/scripts/releaseElement.sh`. It will download the APKs from Buildkite check them and sign them.
- [ ] Wait for the [GitHub action](https://github.com/vector-im/element-android/actions/workflows/build.yml?query=branch%3Amain) to build the `main` branch.
- [ ] (TODO This script has to be updated) *Run the script `~/scripts/releaseElement.sh`. It will download the APKs from Buildkite check them and sign them.*
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, where is this releaseElement.sh script?

Copy link
Member Author

Choose a reason for hiding this comment

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

This script is not shared publicly, but uses scripts that are public and in the repository, like download_buildkite_artifacts.py and sign_apk_unsafe.sh.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mnaturel f752146

Can you check the script on your side?

Copy link
Contributor

Choose a reason for hiding this comment

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

The script seems to work well on my side. I think we need to add a small doc about this script somewhere to explain how to use it.

@@ -3,6 +3,9 @@
# Exit on any error
set -e

echo "Sorry, this script needs to be updated to download APKs from GitHub action. Buildkite is not building APKs anymore."
Copy link
Contributor

Choose a reason for hiding this comment

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

Will the script be updated in another dedicated PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess I will write another script to install from GitHub, but yes, in another PR. I will write it during the next release.

Copy link
Member Author

Choose a reason for hiding this comment

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

(Actually I am working on it right now)

Copy link
Contributor

@mnaturel mnaturel left a comment

Choose a reason for hiding this comment

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

Right now I have this for local build in the splashscreen:

I don't know if this is already the case but, in the case of a CI build, do you know if it would be possible to add the commit hash after the branch name to have more precision about where we are in the branch?

@mnaturel
Copy link
Contributor

@bmarty When testing a build from this PR using the script, I get this info on the splashscreen:

We have HEAD for the branch name but I don't know why. And I cannot find the hash commit bd62fbed in the repo. Do you have an idea of what is happening?

@bmarty bmarty force-pushed the feature/bma/remove_buidkite branch from 12a797f to 211c0c2 Compare November 24, 2022 14:36
@bmarty
Copy link
Member Author

bmarty commented Nov 24, 2022

@mnaturel I have updated the PR to add a docs to use ./tools/install/installFromGitHub.sh.

For the version I have (nearly) the same issue:

image

I will have a look, but I think it should not block this PR.

Copy link
Contributor

@mnaturel mnaturel left a comment

Choose a reason for hiding this comment

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

LGTM.

@sonarqubecloud
Copy link

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

30.0% 30.0% Coverage
0.0% 0.0% Duplication

@bmarty bmarty merged commit 27419f0 into develop Nov 24, 2022
@bmarty bmarty deleted the feature/bma/remove_buidkite branch November 24, 2022 17:51
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.

Migrate off BuildKite
3 participants