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

971 release tag seems wrong #1010

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

BPerlakiH
Copy link
Collaborator

Fixes: #971

I've updated the Readme file and the CD workflow to have our new approach documented and enforced.

@BPerlakiH BPerlakiH added this to the 3.6.0 milestone Oct 17, 2024
@BPerlakiH BPerlakiH linked an issue Oct 17, 2024 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Oct 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.93%. Comparing base (d79d153) to head (3fc98ca).

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1010       +/-   ##
===========================================
+ Coverage   26.32%   37.93%   +11.60%     
===========================================
  Files         114      119        +5     
  Lines        6495     6959      +464     
===========================================
+ Hits         1710     2640      +930     
+ Misses       4785     4319      -466     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

Copy link
Contributor

@kelson42 kelson42 left a comment

Choose a reason for hiding this comment

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

@BPerlakiH I believe this was still not clear to you how I want it to work.

I have fixed the README.md but:

  • The section "Last Step" is probably still wrong
  • The worflow is still wrong for the release.

There is no something like a "post release step", this is one release process which is triggered by the GitHub release and then go to testflight. It's not testflight and then github release.

@BPerlakiH
Copy link
Collaborator Author

Ohh, it is still not clear for me...
This is my understanding of it:
We need to build and upload a binary build for Apple to review.
We do this via CD (either weekly or with the TestFlight tag).
For the AppStore review we select (one of the) binaries already uploaded (usually the latest).
Once the binary is approved, we cannot change it to something else, eg. triggering the Github release -> build new binary -> upload to App Store Connect.
So my understanding is that once the binary (previously built) is approved by Apple, we need to tag it on Github as a release. This would be sufficient enough, but we want a dmg file as well. So we rebuild it from the same commit but only for FTP .dmg file.

@kelson42
Copy link
Contributor

kelson42 commented Oct 19, 2024

For the AppStore review we select (one of the) binaries already uploaded (usually the latest).

No, this has to be one we have released through the usual process. The validation of Apple does not decide what we release. We decide what we release and Apple decides if it goes through to the audience of its App store.

@BPerlakiH
Copy link
Collaborator Author

For the AppStore review we select (one of the) binaries already uploaded (usually the latest).

No, this has to be one we have released through the usual process. The validation of Apple does not decide what we release. We decide what we release and Apple decides if it goes through to the audience of its App store.

Ok, I see your point now. This is how I understand it:

  1. we can create testflight builds as many we like, it won't affect either the public release to the app store or the GitHub release, or the FTP release
  2. for each App Store release, we trigger an appropriate Github release, so for example:
  • we Github release 3.6.0 that creates a github tag, builds everything and uploads it to Apple, Apple reviews it the next day or so
  • if 3.6.0 is rejected
  • we change some code
  • create Github release 3.6.1, that builds everything uploads it to Apple, Apple reviews it, and approves the next day
  • we press the public release button on App Store Connect, so it's available for everyone publicly
  • at this point, we still need a "post App Store release step" to create and upload the .dmg binary to FTP, to make that publicly available and the very same that is available in the App Store.

@BPerlakiH
Copy link
Collaborator Author

BPerlakiH commented Oct 19, 2024

@kelson42 After re-reading your changes I think we are missing some pieces, to get a common understanding of this process.

We can create a release version, eg: 3.6.0. This is stored in git within the code. As time passes we weekly / or on demand create builds for this, that is available to try out. For clarity let's call them release candidates. This possibility of trying them out is available via TestFlight. The idea is that it should be tested on real devices before the public sees it.
Under each release version, we have several builds, eg this is for 3.6.0:

Screenshot 2024-10-20 at 00 59 18

Those builds are uploaded there via our TestFlight CD process. Any of those builds can be picked to be sent to Apple and make it publicly available on the App Store as version 3.6.0. This is what I meant that we usually "pick the latest one".

So far we were following the safest path, that the build (release candidate) we try out is the one we send to Apple. If it's approved, we want to mark it as a "final" release, and as such it should be reflected in GitHub release and as a git tag as well. At the same time, we do want to upload this to FTP so it's available there as well. These 2 last steps is automated, by the formerly called "post Release step":

  1. "visible" as a Github release
  2. uploaded to FTP
    maybe it should have been called 'post App Store step'.

From all of this, what we haven't really practiced so far is creating the proper GitHub release tag, and created too many "testflight" tags with various suffixes.

Now we can turn this around, and do a new build and tag as a GitHub release, which will produce a new build on that list, and retest that, and pick it to be published to the App Store. If all is good, it still requires us to do the FTP step, once it's approved.
With this approach though we might end up with more GitHub release versions, than App Store versions, since we will GitHub release and tag even the ones that were not approved.

Eg: version 1.6.0 is not approved by Apple:

App Store: GitHub:
1.6.1 1.6.1
1.5.0 1.6.0
1.5.0

@BPerlakiH
Copy link
Collaborator Author

BPerlakiH commented Oct 19, 2024

In this whole Apple process of getting from code to public App Store, the release version is fixed from the very beginning. The build number varies though.
We can go against that, and say if Apple rejects a specific version we start all over and create a new version with a patch number higher, that's OK with me. I would still prefer to have matching App Store released versions with GitHub released versions. So that the "release" means it is publicly available both on App Store and via FTP. Unfortunately we cannot do all of it in one go, as the review process interrupts us.

@kelson42
Copy link
Contributor

kelson42 commented Oct 20, 2024

So that the "release" means it is publicly available both on App Store

We primarely release a code (version), all the rest is secondary.

Per definition, if Apple start to refuse releases, we will have more releases than version available publicaly in the App store.

For the moment, dev versions don't have a dedicated name... but this is going to change. But this is not the point of this request.

rgaudin
rgaudin previously approved these changes Nov 1, 2024
Copy link
Member

@rgaudin rgaudin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@rgaudin rgaudin left a comment

Choose a reason for hiding this comment

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

👍

@kelson42
Copy link
Contributor

kelson42 commented Nov 21, 2024

I don't know if this is ready to review but at a first look I don't see how the release event would here push on testflight.... Or it was already doing it in the past?

Release](https://github.com/kiwix/kiwix-apple/releases). Only the
version released (to Testflight) that way can be then sent to
approval to Apple. Once approved by Apple, we can release them to the
AppStore. At the same time, will publish the macOS DMG to our [file
Copy link
Member

Choose a reason for hiding this comment

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

This At the same time is confusing. It should be clear that this is triggered by the Github release event.

@rgaudin
Copy link
Member

rgaudin commented Nov 21, 2024

I don't know if this is ready to review but at a first look I don't see how the release event would here push on testflight.... Or it was already doing it in the past?

Indeed, I thought this was a doc-only PR. What is described in this PR's readme is not what is happening in the workflow. We are setting UPLOAD_TO differently. This will require refactoring of the workflow as it was previously either one or the other

@kelson42
Copy link
Contributor

@BPerlakiH Can you please focus on fixing this? This is a very old PR and I want this working fine before releasing.

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.

Release tag seems wrong
4 participants