Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Conditional foundation import #1704

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

thomasvl
Copy link
Collaborator

No description provided.

@thomasvl thomasvl marked this pull request as ready for review August 28, 2024 20:00
@thomasvl
Copy link
Collaborator Author

@gjcairo fyi

@thomasvl thomasvl requested review from allevato and tbkka August 28, 2024 20:00
Copy link
Contributor

@gjcairo gjcairo left a comment

Choose a reason for hiding this comment

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

LGTM but two doc nits.

Thanks Thomas!

@@ -15,6 +15,22 @@ extension FileDescriptor {
return SwiftProtobufInfo.isBundledProto(file: self)
}

// Returns true if the file will beed to import Foundation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Returns true if the file will beed to import Foundation.
// Returns true if the file will need to import Foundation.

@@ -166,6 +182,22 @@ extension Descriptor {
/// Returns true if the message should use the message set wireformat.
var useMessageSetWireFormat: Bool { return options.messageSetWireFormat }

var needsFoundationImport: Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: can we add the same docs from FileDescriptor above to this?

@thomasvl thomasvl merged commit 237a664 into apple:main Aug 29, 2024
10 checks passed
@thomasvl
Copy link
Collaborator Author

Oops, window didn't reload to show the comment until after I hit merge. Follow up PR coming.

thomasvl added a commit to thomasvl/swift-protobuf that referenced this pull request Aug 29, 2024
thomasvl added a commit to thomasvl/swift-protobuf that referenced this pull request Aug 29, 2024
thomasvl added a commit that referenced this pull request Aug 29, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants