Skip to content

Add a documentation article for @_spi and when to use existentials #1696

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

Merged
merged 4 commits into from
May 24, 2023

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented May 23, 2023

Also remove the Create SwiftSyntax Release.md because it describes how to vendor the parser library and we’re no longer doing that.

Also remove the `Create SwiftSyntax Release.md` because it describes how to vendor the parser library and we’re no longer doing that.
@ahoppen ahoppen requested a review from bnbarham May 23, 2023 18:16
@ahoppen
Copy link
Member Author

ahoppen commented May 23, 2023

@swift-ci Please test

@ahoppen ahoppen changed the title Add a documentation article for @_spi Add a documentation article for @_spi and when to use existentials May 23, 2023
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe usage of double ticks would make sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Updated

@ahoppen ahoppen force-pushed the ahoppen/document-spi branch from 91e12ce to d4d4a70 Compare May 23, 2023 20:15
Copy link
Member

@TTOzzi TTOzzi 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 for adding the detailed document! I have left a few minor comments 🙇‍♂️

@@ -0,0 +1,10 @@
# When to use protocols in SwiftSyntax

Learn when to use protocols value types like ``ExprSyntax`` over protocols like ``ExprSyntaxProtocol``.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't feel like this line (and the one in the other file) add much beyond the title itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree. But docc renders this as the abstract of the article and without this the entire next paragraph becomes the abstract, which looks ugly.

See https://swiftpackageindex.com/apple/swift-syntax/508.0.1/documentation/swiftsyntax/changingswiftsyntax

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah good to know, thanks.

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

Nice, thanks Alex!

@ahoppen ahoppen force-pushed the ahoppen/document-spi branch from 4407c49 to 4d0d98b Compare May 23, 2023 22:52
@ahoppen
Copy link
Member Author

ahoppen commented May 23, 2023

@swift-ci Please test

@ahoppen ahoppen enabled auto-merge May 23, 2023 22:52
@ahoppen
Copy link
Member Author

ahoppen commented May 23, 2023

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 28647fe into swiftlang:main May 24, 2023
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.

4 participants