Skip to content

[email protected] #25

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

[email protected] #25

wants to merge 14 commits into from

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

- cleanup duplicate code path when obtaining app id
@StephenHodgson StephenHodgson requested a review from Copilot April 19, 2025 05:05
@StephenHodgson StephenHodgson marked this pull request as ready for review April 19, 2025 05:05
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 cleans up the duplicate code path for obtaining the app id by removing a redundant local implementation and routing calls through a common utility function.

  • Removed the local getAppId function from src/xcode.ts.
  • Updated references to use GetAppId from AppStoreConnectClient.
  • Adjusted the return type of GetAppId to more accurately reflect its purpose.

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

File Description
src/xcode.ts Removed duplicate getAppId implementation and updated its usage.
src/AppStoreConnectClient.ts Changed GetAppId to return a string and updated how it’s assigned.
Files not reviewed (1)
  • package.json: Language not supported

@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 cleans up duplicate code paths when obtaining the app ID and fixes the SemVer processing for Info.plist updates. Key changes include consolidating app ID retrieval through GetAppId, improving error messages with more context, and updating the Xcode version handling in the workflow.

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 Added validation for project files, enhanced error messaging, and fixed SemVer handling in Info.plist.
src/index.ts Refactored Xcode version selection and installation logic using the new "xcodes installed" command.
src/XcodeProject.ts Removed duplicate appId property to prevent conflicts.
src/AppleCredential.ts Removed the redundant appleId field, aligning with the new appId approach.
src/AppStoreConnectClient.ts Updated GetAppId to return a string and adjusted related logic accordingly.
.github/workflows/validate.yml Adjusted the workflow input to specify a concrete xcode-version.
Files not reviewed (1)
  • package.json: Language not supported
Comments suppressed due to low confidence (1)

src/AppStoreConnectClient.ts:51

  • [nitpick] The function name 'GetAppId' uses PascalCase while many helper functions follow camelCase. For consistency, consider renaming it to 'getAppId' or updating similar functions accordingly.
export async function GetAppId(project: XcodeProject): Promise<string> {

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