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

use Trio specific names and update instructions in testflight.md #297

Merged
merged 6 commits into from
Jun 19, 2024

Conversation

marionbarker
Copy link
Contributor

@marionbarker marionbarker commented Jun 11, 2024

Added to this PR - a modification to fastlane/Fastfile to use Trio specific identifier names. (The identifiers themselves are not modified, just the description.)

  • Add reference to github organization method which simplifies building many apps
  • Add additional information previously only found in the docs.
  • Include the fourth Identifier

This PR is in response to Issue #294.
With the final version, it also answers issue #174.

MikePlante1
MikePlante1 previously approved these changes Jun 11, 2024
Copy link
Contributor

@MikePlante1 MikePlante1 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! I think this one should be merged to dev before 1.0.0 is released.

Here's some screenshots of the preview:

Screenshot 2024-06-11 at 12 52 30 PM Screenshot 2024-06-11 at 12 51 16 PM

@bjornoleh
Copy link
Contributor

If you look at my up to date screenshots from yesterday in #294, I am unsure if this part is accurate, as I am not seeing the FreeAPS names anywhere in my screenshots? Surely the build targets are still called FreeAPS, but I think it will only bring confusion to mention this here in the context of GH builds and what to look for on the Apple developer portal:

Repeat this step for each NAME with associated IDENTIFIER:
* FreeAPS: org.nightscout.TEAMID.trio
* FreeAPS watchkitapp: org.nightscout.TEAMID.trio.watchkitapp
* FreeAPS watchkitapp watchkitextension: org.nightscout.TEAMID.trio.watchkitapp.watchkitextension

@MikePlante1
Copy link
Contributor

If you look at my up to date screenshots from yesterday in #294, I am unsure if this part is accurate, as I am not seeing the FreeAPS names anywhere in my screenshots? Surely the build targets are still called FreeAPS, but I think it will only bring confusion to mention this here in the context of GH builds and what to look for on the Apple developer portal:

Repeat this step for each NAME with associated IDENTIFIER: * FreeAPS: org.nightscout.TEAMID.trio * FreeAPS watchkitapp: org.nightscout.TEAMID.trio.watchkitapp * FreeAPS watchkitapp watchkitextension: org.nightscout.TEAMID.trio.watchkitapp.watchkitextension

yours looks like this because you built via Xcode first:
Xcode

Had you never built with Xcode and only built with GitHub, it would have looked like this (except the last one would just say FreeAPS.):
gh

@bjornoleh
Copy link
Contributor

Ah, right! I expected differences due to Xcode, just didn’t know exactly what to expect, I thought it was only the XC prefix.

Perhaps we should include instructions to search for Trio if there are many identifiers listed? That really clears it up for most.

@marionbarker
Copy link
Contributor Author

@bjornoleh Are you requesting a modification?

Should I put in a table of what the XC version looks like under NAMES? Or will most people be looking at the docs.
Does LiveActivity not get an XC in front of it?

@bjornoleh
Copy link
Contributor

@bjornoleh Are you requesting a modification?

Should I put in a table of what the XC version looks like under NAMES? Or will most people be looking at the docs. Does LiveActivity not get an XC in front of it?

A table with the typical names for Xcode vs Fastlane build would be great.

Unsure about LiveActivity, perhaps this was added from a GitHub build because this feature was added later? I would think so.

@marionbarker
Copy link
Contributor Author

Summary

Do some testing prior to building a table for Identifier Names (Descriptions) for Mac-Xcode vs GitHub Actions (Browser Build) method for Trio.

Question:

Do we really need LiveActivity as an identifier for Browser Build?

Tests with Trio alpha, commit 82f2d01.

I deleted all identifiers (except Trio itself) and renamed the Trio description to be XC bundle_id

When I built with Mac-Xcode - these are the only identifiers on my identifiers page - no sign of LiveActivityExtension:

trio-identifiers-from-mac-xcode

I then ran some actions with my fork of Trio-alpha :

  • Add Identifiers (alpha) - success
    • The previously shown identifiers are there but so is LiveActivity
    • LiveActivity Identifier details has AppGroup row checked
    • Add LoopGroup to the 3 trio identifiers (not Live activity)

trio-identifiers-from-add-identifiers

  • Create Certificates (alpha) - success
  • Build Trio (alpha)
    • I will update this comment after the build action finishes (it is 11 minutes in right now)
    • then I plan to delete the LiveActivity identifier and try again

@marionbarker
Copy link
Contributor Author

I got more information via PM with @dnzxy.
I propose a different solution.

  • I suggest a change to fastlane/Fastfile (patch below) - we could add to this PR (with an edited name) or close this and create a new PR
    • remove the AppGroup from LiveActivity (not needed and is confusing)
    • change the "name" for all identifiers to begin with Trio (so Identifier rows are adjacent and unique)
      • this separates Trio from iAPS Identifier rows
  • Changes of this type in Fastfile require corresponding changes in fastlane/testflight.md, so should all be in one PR
  • The documentation would then be changed accordingly with a PR to trio-docs

proposed-trio-identifiers

Patch for Fastfile:
trio-specific-identifiers.patch

@marionbarker
Copy link
Contributor Author

Testing of Proposed Name Change

I created a branch, trio-specific-identifiers, at loopandlearn/Trio with this modification wrt nightscout/Trio alpha:

I am in the process of testing this single Fastfile change after deleting Identifiers, Certificates and Match-Secrets.

@marionbarker
Copy link
Contributor Author

If this modification is added to Trio, then Issue #174 can be closed.

@marionbarker
Copy link
Contributor Author

Test successful with proposed change (see graphic).
(Forgot to mark on graphic) Delete all Trio XXX Identifiers

  1. Clear previous certs: delete Match-Secrets and revoke distribution certificates
  2. Successful Action: Add Identifiers
  3. Add Trio App Group to Trio Watch and Trio WatchKit Extension
  4. Successful Action: Create Certificates
  5. Successful Action: Build Trio

Open questions:

  • Consensus on desired names
  • Decision whether to add Trio App Group to LiveActivity in case it is needed in the future

trio-specific-identifiers-test

@bjornoleh bjornoleh linked an issue Jun 18, 2024 that may be closed by this pull request
@marionbarker marionbarker changed the title update the testflight.md file use Trio specific names and update instructions in testflight.md Jun 18, 2024
@marionbarker
Copy link
Contributor Author

This is ready for review.

  • I confirmed that the LiveActivity Identifier does not require a shared app group
  • Please test the instructions in testflight.md using the modified Fastfile in this PR
  • For testing, you can delete all except the Trio identifier and then just Add Identifiers and edit for the App Group
  • I also deleted identifiers and did Mac-Xcode build to capture that version for Identifiers

fastlane/testflight.md Outdated Show resolved Hide resolved
bjornoleh
bjornoleh previously approved these changes Jun 19, 2024
Copy link
Contributor

@bjornoleh bjornoleh 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 and works well! The only thing I did not test was the optional renaming.

Copy link
Contributor

@dnzxy dnzxy left a comment

Choose a reason for hiding this comment

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

Approving and merging.

@dnzxy dnzxy merged commit 6fc084d into nightscout:alpha Jun 19, 2024
@marionbarker marionbarker deleted the update_browser_build_directions branch June 19, 2024 19:31
@MikePlante1 MikePlante1 mentioned this pull request Jun 25, 2024
mountrcg pushed a commit to mountrcg/Trio that referenced this pull request Jun 27, 2024
…ild_directions

use Trio specific names and update instructions in testflight.md
@MikePlante1 MikePlante1 mentioned this pull request Jun 27, 2024
mountrcg pushed a commit to mountrcg/Trio that referenced this pull request Aug 1, 2024
mountrcg pushed a commit to mountrcg/Trio that referenced this pull request Aug 1, 2024
…w through add meal button in bolus view (nightscout#297)

(cherry picked from commit b0a7bf4)
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.

Identifier Confusion
4 participants