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

Fix build errors in Xcode16-RC #141

Merged
merged 1 commit into from
Sep 12, 2024
Merged

Conversation

simorgh3196
Copy link
Contributor

I have fixed an error that occurs when building with Xcode 16 Release Candidate.
This change is also included in #140, but I wanted to use it immediately, so I created a small separate PR.

Changes

  • Removed unnecessary @retroactive declarations

Copy link
Owner

@giginet giginet left a comment

Choose a reason for hiding this comment

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

Thank you!

Could you fix the commented points?

Comment on lines -13 to -25
extension BuildConfiguration: @retroactive ExpressibleByArgument {
public init?(argument: String) {
switch argument.lowercased() {
case "debug":
self = .debug
case "release":
self = .release
default:
return nil
}
}
}

Copy link
Owner

Choose a reason for hiding this comment

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

This extension was also required for Swift 5 & 6.

Please move L34 to the outside of the preprocessor block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/giginet/Scipio/blob/c062eeb00bfd62618a7e316cab725a318262c77b/Sources/scipio/Arguments.swift

The BuildConfiguration extension is already outside the preprocessor block.
If my perception is wrong, I would like to know!

Copy link
Owner

Choose a reason for hiding this comment

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

oops. It's correct!

Copy link
Owner

@giginet giginet left a comment

Choose a reason for hiding this comment

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

LGTM!

These same changes are applied on #140, but we should merge this PR first.
I'll ask the author to rebase this PR.

Comment on lines -13 to -25
extension BuildConfiguration: @retroactive ExpressibleByArgument {
public init?(argument: String) {
switch argument.lowercased() {
case "debug":
self = .debug
case "release":
self = .release
default:
return nil
}
}
}

Copy link
Owner

Choose a reason for hiding this comment

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

oops. It's correct!

@giginet giginet merged commit 36ea7c8 into giginet:main Sep 12, 2024
4 checks passed
@simorgh3196 simorgh3196 deleted the support-xcode16-rc branch September 12, 2024 08:27
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