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 GH Actions for CI #262

Merged
merged 13 commits into from
Dec 7, 2023
Merged

Use GH Actions for CI #262

merged 13 commits into from
Dec 7, 2023

Conversation

cwillisf
Copy link
Contributor

@cwillisf cwillisf commented Dec 6, 2023

Resolves

Resolves ENG-46

Proposed Changes

Implement GitHub Actions CI for Scratch Link and remove CCI config. The actual build steps are extracted into reusable actions to make the main ci.yml more DRY.

Supporting changes:

  • Delete the maximum effort install script for Visual Studio Mac since GHA Mac images come with it preinstalled.
  • Allow Xcode to make some adjustments to the macOS project files. I don't fully understand the changes it made, but it made the build work, so...

Note that the calculated version number looks incorrect; I believe that's a limitation of using GitVersion for that. I might switch the build-time version calculation to use the value from the package.json file, but that'll be a separate PR.

These were accidentally deleted when shuffling macOS setup steps to an
`macos-setup/action.yml` and back. These variables make Xcode behavior
consistent across different GHA images.
@cwillisf
Copy link
Contributor Author

cwillisf commented Dec 6, 2023

Sorry, I forgot to finish the macOS build steps (notarization, etc.) so this isn't really ready for review. The option to convert this PR to a draft isn't showing up for me, so I'll remove the review requests for now. I'll re-request when it's ready.

Comment on lines +3 to +11
inputs:
configuration:
description: The configuration to build, like "Debug"
required: true
default: Debug
artifact_tag:
description: A string to tag the build artifact, like "Debug" or "MAS"
required: false
default: ""
Copy link

Choose a reason for hiding this comment

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

Neat! Did not know about inputs. 💪

Comment on lines +81 to +82
AC_USERNAME: ${{ secrets.AC_USERNAME }}
AC_PASSWORD: ${{ secrets.AC_PASSWORD }}
Copy link

Choose a reason for hiding this comment

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

Are the AC_USERNAME and AC_PASSWORD the same for scratch-desktop? I have these set in repository secrets as well, but if they are the same we should probably move them to org secrets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that would make sense!

@@ -65,7 +65,7 @@ jobs:
run:
shell: bash # even on Windows, unless otherwise specified
env:
SCRATCH_SHOULD_SIGN: "NO" # TODO
SCRATCH_SHOULD_SIGN: "YES" # TODO
MATCH_STORAGE_MODE: git
Copy link

@delasare delasare Dec 7, 2023

Choose a reason for hiding this comment

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

This is also in secrets if you want to use the variable FL_STORAGE_MODE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha, yeah, I had that in a local secret for a while, but it made GHA redact all instances of git in the logs, which was just silly. We could add it as a var instead of a secret, but having the GIT_URL secret as well means changing the storage mode by itself wouldn't be enough to reconfigure it. 🤷

Copy link

@delasare delasare left a comment

Choose a reason for hiding this comment

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

Just a few comments and suggestions! Looks good.

@cwillisf cwillisf merged commit ff2e736 into develop Dec 7, 2023
4 of 5 checks passed
@cwillisf cwillisf deleted the gh-actions branch December 7, 2023 17:43
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