Skip to content

Conversation

StephenHodgson
Copy link
Member

@StephenHodgson StephenHodgson commented Apr 19, 2025

  • cleanup duplicate code path when obtaining app id
  • fix SemVer when processing info.plist
  • enhancing Xcode version selection logic
  • bundle version auto-increment logic improvements

@StephenHodgson StephenHodgson marked this pull request as ready for review April 23, 2025 13:51
@StephenHodgson StephenHodgson requested a review from a team as a code owner April 23, 2025 13:51
@StephenHodgson StephenHodgson requested a review from Copilot April 25, 2025 00:38
Copy link

@Copilot Copilot AI left a 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> {

Copy link

@Copilot Copilot AI left a 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

@StephenHodgson StephenHodgson requested a review from Copilot April 29, 2025 17:02
Copy link

@Copilot Copilot AI left a 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

@StephenHodgson StephenHodgson requested a review from Copilot April 29, 2025 17:45
Copy link

@Copilot Copilot AI left a 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}!`); }

@StephenHodgson StephenHodgson requested a review from Copilot April 29, 2025 20:58
Copy link

@Copilot Copilot AI left a 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> {

@StephenHodgson StephenHodgson requested a review from Copilot May 5, 2025 13:56
Copy link

@Copilot Copilot AI left a 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'

@StephenHodgson StephenHodgson requested a review from Copilot May 5, 2025 14:00
Copy link

@Copilot Copilot AI left a 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,

@StephenHodgson StephenHodgson merged commit 3898127 into main May 5, 2025
16 of 18 checks passed
@StephenHodgson StephenHodgson deleted the development branch May 5, 2025 16:00
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.

1 participant