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

Fastlane/GH build improvements: Add sync action and keepalive action, #28

Merged
merged 5 commits into from
Mar 30, 2024

Conversation

bjornoleh
Copy link
Contributor

@bjornoleh bjornoleh commented Mar 22, 2024

and align with Loop dev improvements, as previously added to iAPS in PR 46.

Copied as of iAPS 540ae95, does not include e2e77a8.

… align with Loop dev improvements, as previously added to iAPS in PR 46
@bjornoleh bjornoleh requested a review from dnzxy March 22, 2024 20:44
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.

We need to update all runner actions by GH to v4 (artifact and checkout ones) for Node runner compliance. The sync action was updated after a PR I submitted and latest should be v3.4.1 IIRC. See also pending PR here LoopKit/LoopWorkspace#120

Rest LGTM so far.
Anything that may have been missed, @marionbarker ?

.github/workflows/build_iAPS.yml Outdated Show resolved Hide resolved
if: |
needs.check_alive_and_permissions.outputs.WORKFLOW_PERMISSION == 'true' &&
(vars.SCHEDULED_BUILD != 'false' || vars.SCHEDULED_SYNC != 'false')
uses: actions/checkout@v3
Copy link
Contributor

Choose a reason for hiding this comment

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

@v4 required here for nodeJS requirements

.github/workflows/build_iAPS.yml Outdated Show resolved Hide resolved
if: |
needs.check_alive_and_permissions.outputs.WORKFLOW_PERMISSION == 'true' &&
vars.SCHEDULED_SYNC != 'false'
uses: actions/checkout@v3
Copy link
Contributor

Choose a reason for hiding this comment

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

@v3.4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's v4, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, definitely. Sorry, brain fart.

.github/workflows/build_iAPS.yml Outdated Show resolved Hide resolved
@marionbarker
Copy link
Contributor

Once this is updated, I will test the build.

- use actions/checkout@v4
- use aormsby/Fork-Sync-With-Upstream-action@v3.4.1
- comment out "Select Xcode version" step to use the default Xcode (15.0.1) in macos-13 runner

based on https://github.com/LoopKit/LoopWorkspace/pull/120/files by @dnzxy
@bjornoleh
Copy link
Contributor Author

We need to update all runner actions by GH to v4 (artifact and checkout ones) for Node runner compliance. The sync action was updated after a PR I submitted and latest should be v3.4.1 IIRC. See also pending PR here LoopKit/LoopWorkspace#120

Rest LGTM so far. Anything that may have been missed, @marionbarker ?

Should be good now, please have a look, and test if possible :-) Thanks!

@marionbarker
Copy link
Contributor

Testing this now.
Please update the Fastlane/testflight.md file. It was not included in the changes in this PR.

Starting at instructions

## Setup Github iAPS repository

@marionbarker
Copy link
Contributor

summary:

  • test failed

test details

For those who have access to my fork of the private repo (happy to provide it to others by request), the actions can be viewed at:

I did not have an alive branch. Tried creating one, but as expected - not good enough.

This needs to be updated - note looking for iAPS/branches not Open-iAPS branches:

Also - the current repo should be whatever the user calls it - we ran into a problem with LoopWorkspace where the user renamed her fork - which should be allowed - but then the build failed because action was looking for username/LoopWorkspace instead of username/my-workspace-name.

##[group]Run if [[ "$(gh api -H "Accept: application/vnd.github+json" /repos/marionbarker/iAPS/branches | jq --raw-output 'any(.name=="alive")')" == "true" ]]; then
  env:
    UPSTREAM_REPO: Artificial-Pancreas/iAPS
    UPSTREAM_BRANCH: fastlane-test-submodules
    TARGET_BRANCH: fastlane-test-submodules
    ALIVE_BRANCH: alive
    ALIVE_BRANCH_EXISTS: false
    GITHUB_TOKEN: ***

branch details

My branch fastlane-test-submodules was created by applying a patch from this PR to my branch mdb_testing.
And mdb_testing is created from the Nightscout:loopkit_dev_submodules branch and applying updates found at loopandlearn for these submodules:

  • G7SensorKit
  • OmniBLE
  • OmniKit

@dnzxy
Copy link
Contributor

dnzxy commented Mar 24, 2024

Yup the alive branch creation is broken. It might actually be, that this is somewhat inaccessible to build (with alive stuff) and test via private repos. I'm about to leave for Valencia early tomorrow, so will definitely not be able to look into this.

@bjornoleh the Create alive branch sub-step is the culprit

Run # Get ref for Artificial-Pancreas/iAPS:dev
gh: Resource not accessible by integration (HTTP 403)
{"message":"Resource not accessible by integration","documentation_url":"https://docs.github.com/rest/git/refs#create-a-reference"}
Error: Process completed with exit code 1.

@bjornoleh
Copy link
Contributor Author

bjornoleh commented Mar 24, 2024

summary:

  • test failed

test details

For those who have access to my fork of the private repo (happy to provide it to others by request), the actions can be viewed at:

I did not have an alive branch. Tried creating one, but as expected - not good enough.

This needs to be updated - note looking for iAPS/branches not Open-iAPS branches:

Also - the current repo should be whatever the user calls it - we ran into a problem with LoopWorkspace where the user renamed her fork - which should be allowed - but then the build failed because action was looking for username/LoopWorkspace instead of username/my-workspace-name.

...

  env:
    UPSTREAM_REPO: Artificial-Pancreas/iAPS
...

Thanks for testing. I think your findings should be fixed now.

Edit: also -f pushed the testflight.md changes now

- UPSTREAM_REPO: nightscout/Open-iAPS
- Search for and create `alive` branch by use of `${{ github.repository }}` context, instead of
`${{ github.repository_owner }}/iAPS`
- Create `alive` branch from `main` instead of `dev`
- do not run the upstream sync action on the upstream repository (owner = nightscout)
- update testflight.md with the Open-iAPS name
@marionbarker
Copy link
Contributor

summary

  • partial success
  • failures probably due to private repo (please evaluate the failed and success links below)

test details

branch details

See earlier comment. Updated using the following steps.

git diff  e07994ba..9dd2523a > OiAPS-28-browserbuild_update.patch
git apply OiAPS-28-browserbuild_update.patch
git add .
git commit -am "update PR 28 e07994ba..9dd2523a"

Confirm Xcode builds with no issues.

Make mods to alive branch does not exist directly on GitHub based on PM from bjorn. Made no difference.
Note that when alive branch does exist, the modification in marionbarker-patch-1 is not used.

- Authenticate the curl API request by GH_PAT according to
https://docs.github.com/de/rest/authentication/authenticating-to-the-rest-api?apiVersion=2022-11-28#authenticating-in-a-github-actions-workflow-using-curl
Thank you @dnzxy for the hint!

This is a temporary requirement until the repositories are made public.
@marionbarker
Copy link
Contributor

Tested and succeeded with most recent update.
I did not have an alive branch and it succeeded in creating one.

https://github.com/marionbarker/Open-iAPS/actions/runs/8428292151

@bjornoleh bjornoleh requested a review from dnzxy March 26, 2024 06:39
@bjornoleh
Copy link
Contributor Author

LGTM! 😄

@bjornoleh
Copy link
Contributor Author

Merging this now, it has been well tested.

@bjornoleh bjornoleh merged commit 2b4e28e into nightscout:dev Mar 30, 2024
@MikePlante1 MikePlante1 mentioned this pull request Mar 30, 2024
mountrcg pushed a commit to mountrcg/Trio that referenced this pull request Aug 1, 2024
* move basal to bottom, remove background rectangle of main chart

* add selection popover

* refactoring of main chart code

* Various fixes

* fix conversion to mmol/L with custom y axis scaling, clean up

* fixes for mmol/L for popover

* workaround for crash on <iOS 17

* move external insulin checkbox to bolus section to avoid scrolling

* Small UI fixes

* change treatments button text to be more descriptive when only adding carbs

* fix not clickable edges of treatment button

* change 'insulin' string to 'treatments' in history view when in 2 tab view

* end loading animation when glucose data is stale

---------

Co-authored-by: dnzxy <d.c.cengiz@googlemail.com>
Co-authored-by: Andreas Stokholm <andreas@stokholm.me>
Sjoerd-Bo3 pushed a commit to Sjoerd-Bo3/Trio that referenced this pull request Aug 3, 2024
Sjoerd-Bo3 pushed a commit to Sjoerd-Bo3/Trio that referenced this pull request Aug 3, 2024
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.

3 participants