-
Notifications
You must be signed in to change notification settings - Fork 2
unity-xcode-builder@v1.2.1 #17
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
Conversation
- cleanup duplicate code path when obtaining app id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the unity-xcode-builder tool to clean up duplicate code paths and fix SemVer processing in the info.plist. The changes include refactoring how the app ID is obtained, improved error messages regarding project lookup, and adjustments in logging levels and variable naming in the Xcode version selection flow.
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/xcode.ts | Added GetAppId import and updated error messages for project lookup; adjusted logging level for auto-incremented bundle version. |
src/index.ts | Refactored the Xcode version selection and installation flow with clearer exit code handling. |
src/XcodeProject.ts | Removed redundant appId field duplication. |
src/AppleCredential.ts | Removed unused appleId property to align with new appId usage. |
src/AppStoreConnectClient.ts | Updated GetAppId signature to return a string and adjusted its usage accordingly. |
Files not reviewed (1)
- package.json: Language not supported
Comments suppressed due to low confidence (2)
src/index.ts:26
- [nitpick] The variable name 'xcodeVersionOutput' is declared more than once in this function, which may lead to confusion. Consider renaming one of the instances to clarify its distinct purpose.
let xcodeVersionOutput = '';
src/AppStoreConnectClient.ts:51
- [nitpick] The function name 'GetAppId' now returns a string (an app ID) rather than an updated project, which could be confused by consumers expecting the previous behavior. Consider updating documentation or renaming the function to reflect that it returns only the app ID.
export async function GetAppId(project: XcodeProject): Promise<string> {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
The purpose of this PR is to clean up duplicate code for obtaining the app ID, fix SemVer handling in the Info.plist processing, enhance Xcode version selection logic, and improve the bundle version auto-increment mechanism.
- Removed duplicate code paths and redundant properties for app identifier.
- Updated file reading permissions and type conversions for bundle version handling.
- Refactored GetAppId and GetLatestBundleVersion functions; adjusted auto-increment and logging behavior.
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/xcode.ts | Streamlined project path search and improved Info.plist SemVer validation & auto-increment logic. |
src/index.ts | Enhanced Xcode version selection and error handling during version validation. |
src/XcodeProject.ts | Changed bundleVersion type to string and introduced autoIncrementBuildNumber and appId. |
src/AppleCredential.ts | Removed unused appleId property. |
src/AppStoreConnectClient.ts | Updated GetAppId and GetLatestBundleVersion return types; refactored UpdateTestDetails. |
Files not reviewed (1)
- package.json: Language not supported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the unity-xcode-builder logic to clean up duplicate code paths, fix the SemVer parsing for Info.plist, enhance Xcode version selection logic, and improve bundle version auto-increment.
- Removed duplicate code and improved error messages when resolving the project path.
- Updated Info.plist handling to correctly extract, update, and validate version strings.
- Refined Xcode version selection and bundle version auto-increment logic for better consistency.
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/xcode.ts | Enhancements in Info.plist processing, error messaging, and bundle version auto-increment changes. |
src/index.ts | Revamped Xcode version selection logic including installed version detection and validation. |
src/XcodeProject.ts | Changed bundleVersion type from number to string and added autoIncrementBuildNumber property. |
src/AppleCredential.ts | Removed the redundant appleId field to avoid confusion. |
src/AppStoreConnectClient.ts | Adjustments in GetAppId and GetLatestBundleVersion return types and UpdateTestDetails signature. |
Files not reviewed (1)
- package.json: Language not supported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refines the Xcode build process by cleaning up duplicate code paths, fixing SemVer processing for info.plist, improving the Xcode version selection logic, and enhancing the bundle version auto-increment feature. Key changes include:
- Improved error messaging and file detection in project detail acquisition.
- Updated Xcode version handling in the main entry point.
- Transitioning bundleVersion from a number to a string type and adjusting related auto-increment logic.
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/xcode.ts | Improved project file detection and enhanced error messages when processing Info.plist. |
src/index.ts | Refactored Xcode version selection logic including installation and strict version matching checks. |
src/XcodeProject.ts | Changed bundleVersion type from number to string and added the autoIncrementBuildNumber property. |
src/AppleCredential.ts | Removed the deprecated appleId property. |
src/AppStoreConnectClient.ts | Updated GetAppId and GetLatestBundleVersion functions to return and handle string types appropriately. |
Files not reviewed (1)
- package.json: Language not supported
Comments suppressed due to low confidence (1)
src/index.ts:89
- The strict comparison between the requested Xcode version and the selected version may be too strict if the installed version includes additional identifiers; consider allowing acceptable variations to prevent unnecessary errors.
if (xcodeVersionString !== selectedXcodeVersionString) { throw new Error(`Selected Xcode version ${selectedXcodeVersionString} does not match requested version ${xcodeVersionString}!`); }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the unity-xcode-builder to v1.2.1 with several improvements and bug fixes. Key changes include cleaning up duplicate code paths when obtaining the app identifier, fixing SemVer processing for info.plist updates, enhancing Xcode version selection logic, and refining the bundle version auto‐increment logic.
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/xcode.ts | Updates include improved error messaging for project resolution and refined auto-increment logic for bundle versions. |
src/index.ts | Enhancements in Xcode version selection with added validations and improved error messages. |
src/XcodeProject.ts | Updated bundleVersion type from number to string and addition of an autoIncrementBuildNumber flag. |
src/AppleCredential.ts | Removed the appleId property to shift responsibility to XcodeProject.appId. |
src/AppStoreConnectClient.ts | Adjusted GetAppId and GetLatestBundleVersion to return appropriately typed values. |
.github/workflows/validate.yml | Updated workflow step referencing the Unity setup action to use the development branch. |
Files not reviewed (1)
- package.json: Language not supported
Comments suppressed due to low confidence (2)
src/index.ts:93
- [nitpick] The error message thrown when the selected Xcode version does not match the requested version might benefit from additional context or a remedial suggestion to help diagnose version discrepancies.
if (xcodeVersionString !== selectedXcodeVersionString) {
src/AppStoreConnectClient.ts:69
- Since the return type of GetLatestBundleVersion has changed from number to string | null, please ensure that all consumers properly convert or handle the version value during auto-increment operations, and update any associated documentation.
export async function GetLatestBundleVersion(project: XcodeProject): Promise<string | null> {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the unity-xcode-builder by simplifying the code paths for obtaining the app ID, refining the SemVer handling for Info.plist, enhancing Xcode version selection logic, and refining the auto-increment logic for the bundle version.
- Removed duplicate logic for app ID retrieval and improved error messages when locating the Xcode project.
- Refactored Info.plist handling to strictly parse SemVer and upgraded Xcode version selection to include installation and selection of missing versions.
- Updated data types in XcodeProject and adjusted workflow configuration by removing the hard-coded architecture.
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/xcode.ts | Enhanced error logging and bundle version auto-increment logic with refined SemVer processing. |
src/index.ts | Improved Xcode version selection, installation, and validation logic. |
src/XcodeProject.ts | Updated bundleVersion to use a string type and added an autoIncrementBuildNumber flag. |
src/AppleCredential.ts | Removed the obsolete appleId property. |
src/AppStoreConnectClient.ts | Refactored GetAppId and GetLatestBundleVersion with stricter typing and added detailed logging. |
.github/workflows/validate.yml | Removed explicit architecture specification. |
Files not reviewed (1)
- package.json: Language not supported
Comments suppressed due to low confidence (1)
.github/workflows/validate.yml:49
- [nitpick] Removal of the explicit 'arm64' architecture line may affect build behavior on specific environments; please verify that the environment can correctly infer the intended architecture.
architecture: 'arm64'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses duplicate code cleanup, improves SemVer processing for the Info.plist, enhances Xcode version selection logic, and refines the bundle version auto-increment behavior.
- Removes redundant methods for obtaining the app ID and consolidates version processing logic.
- Updates type definitions and improves error messaging during version selection and build auto-increment.
- Adjusts workflow settings to align with current build platform requirements.
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/xcode.ts | Updated project detail retrieval and version handling logic. |
src/index.ts | Enhanced Xcode version selection and validation logic. |
src/XcodeProject.ts | Modified bundleVersion type and added autoIncrementBuildNumber flag. |
src/AppleCredential.ts | Removed unused appleId property to consolidate credential handling. |
src/AppStoreConnectClient.ts | Refactored GetAppId, polling, and localization update logic. |
.github/workflows/validate.yml | Removed architecture setting for arm64. |
Files not reviewed (1)
- package.json: Language not supported
Comments suppressed due to low confidence (2)
.github/workflows/validate.yml:49
- [nitpick] The removal of the 'architecture' setting is noted; please confirm that the workflow still properly handles builds targeting arm64.
architecture: 'arm64'
src/XcodeProject.ts:13
- Changing 'bundleVersion' from a number to a string represents a breaking change; ensure that all downstream consumers of XcodeProject are updated accordingly and document the change clearly.
bundleVersion: string,
Uh oh!
There was an error while loading. Please reload this page.