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 app #8441

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Remove app #8441

wants to merge 3 commits into from

Conversation

jfx2006
Copy link
Member

@jfx2006 jfx2006 commented Oct 30, 2024

Split the publish_release job into pkginfo, publish_release_github, and publish_release_playstore jobs in anticipation of adding a publish_ftpmo job which requires different permissions.

This also discontinues the use of actions/create-github-app-token@v1 for publish_github and replaces it with contents: 'write' permission.

There are no associated changes to the environments to go with this change.

@jfx2006 jfx2006 requested a review from kewisch October 30, 2024 14:45
Copy link
Member

@kewisch kewisch left a comment

Choose a reason for hiding this comment

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

Are any other changes needed to setup_release_automation, or is there no change since you didn't change the environment?

A few early comments, I'll need to take a closer look at some point.

VERSION_CODE: ${{ steps.pkginfo.outputs.VERSION_CODE }}
APPLICATION_ID: ${{ steps.pkginfo.outputs.APPLICATION_ID }}
app_sha: ${{ steps.shanotes.outputs.app_sha }}
app_github_notes: ${{ steps.shanotes.outputs.app_github_notes }}
Copy link
Member

Choose a reason for hiding this comment

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

Let's use a consistent format with outputs

Copy link
Member

Choose a reason for hiding this comment

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

Also, if you are using a Matrix job, the outputs will overwrite each other, so you only get one random set of variables.

If we need all of these later, it might make sense that instead of picking them from the apk/aab, we generate them from the sources. We have most of this already in the appinfo step early on, we just need to put it into the right outputs.

@@ -674,29 +657,93 @@ jobs:

cat $GITHUB_OUTPUT

- name: Rename release assets
Copy link
Member

Choose a reason for hiding this comment

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

Renaming them was mainly to make sure we have sensible names in the steps such as release assets and artifact names. I don't think we should have a separate artifact to differentiate renamed vs not renamed.


publish_summary:
name: Publish Release Play Store
Copy link
Member

Choose a reason for hiding this comment

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

Name needs changing

publish_release_playstore:
name: Publish Release Play Store
needs: [publish_hold, dump_config, release_commit, pkginfo]
if: ${{ !failure() && !cancelled() }} # Run if previous step is skipped
Copy link
Member

Choose a reason for hiding this comment

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

We can skip this whole job if the skipGooglePlay input is set.

@jfx2006
Copy link
Member Author

jfx2006 commented Nov 1, 2024

Are any other changes needed to setup_release_automation, or is there no change since you didn't change the environment?
There should not be any changes needed.

@jfx2006 jfx2006 marked this pull request as draft November 4, 2024 21:47
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.

2 participants