Pp 2343 enable swift lint across entire mono repo#1064
Conversation
…script to run it locally or in the CI/CD
…ate config file path
…ith proper parameter alignment.
… lines with proper parameter alignment." This reverts commit 681a0f2.
ValentinaIancu-Gini
had a problem deploying
to
firebase-manual-deploy
GitHub Actions
Error
There was a problem hiding this comment.
Pull request overview
Enables SwiftLint usage across the mono-repo by introducing a shared root configuration and wiring SwiftLint into Xcode build phases for example projects.
Changes:
- Added a repo-level
.swiftlint.ymlwith included/excluded paths and rule configuration. - Added a reusable
scripts/swiftlint.shrunner supporting default/strict/fix modes. - Added/updated SwiftLint shell script build phases in BankSDK and HealthSDK example Xcode projects to use the root config.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
scripts/swiftlint.sh |
Adds a CLI entrypoint to run SwiftLint consistently from the repo root. |
HealthSDK/GiniHealthSDKExample/GiniHealthSDKExample.xcodeproj/project.pbxproj |
Adds a SwiftLint build phase pointing to the root .swiftlint.yml. |
BankSDK/GiniBankSDKExample/GiniBankSDKExample.xcodeproj/project.pbxproj |
Adds SwiftLint build phases / updates existing one to use the root .swiftlint.yml; renames a build phase. |
.swiftlint.yml |
Introduces the shared SwiftLint configuration and scope (included paths). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # ── Locate SwiftLint ────────────────────────────────────────────────────────── | ||
| if which swiftlint > /dev/null 2>&1; then | ||
| SWIFTLINT="swiftlint" | ||
| elif which mint > /dev/null 2>&1 && mint which swiftlint > /dev/null 2>&1; then | ||
| SWIFTLINT="mint run swiftlint swiftlint" | ||
| else |
There was a problem hiding this comment.
This script stores a multi-word command in SWIFTLINT (e.g. mint run ...) and later executes it via $SWIFTLINT ..., which relies on word-splitting and is fragile. Consider using a Bash array (e.g. SWIFTLINT=(mint run …)) or a small wrapper function, and prefer command -v over which for tool detection.
| name = SwiftLint; | ||
| outputFileListPaths = ( | ||
| ); | ||
| outputPaths = ( | ||
| ); | ||
| runOnlyForDeploymentPostprocessing = 0; | ||
| shellPath = /bin/sh; | ||
| shellScript = "if test -d \"/opt/homebrew/bin/\"; then\n PATH=\"/opt/homebrew/bin/:${PATH}\"\nfi\nif test -d \"$HOME/.mint/bin\"; then\n PATH=\"$HOME/.mint/bin:${PATH}\"\nfi\nexport PATH\nif which swiftlint >/dev/null; then\n swiftlint --config \"${SRCROOT}/../../.swiftlint.yml\"\nelse\n echo \"warning: SwiftLint not installed, download from https://github.com/realm/SwiftLint\"\nfi\n"; |
There was a problem hiding this comment.
SwiftLint invocation logic is duplicated inline in multiple Xcode project build phases even though scripts/swiftlint.sh exists. Pointing the Xcode build phase(s) at the shared script would reduce drift (e.g. when updating PATH/config options) and make SwiftLint behavior consistent across the mono-repo.
| runOnlyForDeploymentPostprocessing = 0; | ||
| shellPath = /bin/sh; | ||
| shellScript = "if test -d \"/opt/homebrew/bin/\"; then\n PATH=\"/opt/homebrew/bin/:${PATH}\"\nfi\nif test -d \"$HOME/.mint/bin\"; then\n PATH=\"$HOME/.mint/bin:${PATH}\"\nfi\nexport PATH\nif which swiftlint >/dev/null; then\n swiftlint --config \"${SRCROOT}/../../.swiftlint.yml\"\nelse\n echo \"warning: SwiftLint not installed, download from https://github.com/realm/SwiftLint\"\nfi\n"; | ||
| }; |
There was a problem hiding this comment.
SwiftLint build phase script is duplicated inline even though scripts/swiftlint.sh exists at the repo root. Consider calling the shared script from this build phase to keep PATH/config/tool-resolution logic consistent across projects/targets.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if which swiftlint > /dev/null 2>&1; then | ||
| SWIFTLINT="swiftlint" | ||
| elif which mint > /dev/null 2>&1 && mint which swiftlint > /dev/null 2>&1; then | ||
| SWIFTLINT="mint run swiftlint swiftlint" | ||
| else | ||
| echo "❌ SwiftLint not found." | ||
| echo " Install via Homebrew: brew install swiftlint" | ||
| echo " Install via Mint: mint install realm/SwiftLint" |
There was a problem hiding this comment.
The Mint fallback command (mint run swiftlint swiftlint) doesn’t match the installation instructions just below (mint install realm/SwiftLint). As written, this may fail because mint run typically expects the repository name (e.g. realm/SwiftLint). Align the mint run invocation with the documented install source so the fallback works reliably.
| 289235732F48F3090052152A /* SwiftLint */ = { | ||
| isa = PBXShellScriptBuildPhase; | ||
| buildActionMask = 2147483647; | ||
| files = ( | ||
| ); | ||
| inputFileListPaths = ( |
There was a problem hiding this comment.
This SwiftLint build phase (ID 289235...) is missing alwaysOutOfDate = 1 while the other SwiftLint phases in this project have it. That inconsistency can lead to SwiftLint being skipped depending on Xcode’s dependency analysis settings. Consider making the setting consistent across targets (or explicitly configuring input/output paths if you want dependency-based execution).
ValentinaIancu-Gini
had a problem deploying
to
firebase-manual-deploy
GitHub Actions
Failure
qnaveed87
left a comment
qnaveed87
left a comment
There was a problem hiding this comment.
Looks good to me
Thank yo foe the good work @ValentinaIancu-Gini
Pull Request Description
Enables SwiftLint usage across the mono-repo by introducing a shared root configuration and wiring SwiftLint into Xcode build phases for example projects.
Changes:
.swiftlint.ymlwith included/excluded paths and rule configuration.scripts/swiftlint.shrunner supporting default/strict/fix modes.