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

[cxx-interop] add initial 'Mixing Swift and C++' docu… #299

Merged
merged 59 commits into from
Jun 5, 2023

Conversation

hyp
Copy link
Contributor

@hyp hyp commented May 5, 2023

…mentation guide

Draft page preview

Github markdown rendered index page:
https://github.com/hyp/swift-org-website/blob/interop-doc-hello-world/documentation/cxx-interop/index.md

Motivation:

This is an initial skeleton page for the planned documentation guide for how the Swift and C++ interoperability Swift language feature.
We are planning to use initial skeleton page PR to discuss documentation more in depth with the C++ interoperability workgroup, and also present our proposal for documenting Swift and C++ interoperability to the the website workgroup as well.
This PR will be updated until we get to the point where we have some initial documentation for this feature that can be put out on the website.

Modifications:

Added documentation/cxx-interop page.

Result:

Swift will have an initial guide for how to use Swift and C++ interoperability.

@hyp
Copy link
Contributor Author

hyp commented May 5, 2023

initial discussion on the forums is here: https://forums.swift.org/t/documenting-support-for-c-interoperability/64828/1

Copy link
Contributor

@zoecarver zoecarver left a comment

Choose a reason for hiding this comment

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

This is awesome, thanks Alex!

@hyp hyp force-pushed the interop-doc-hello-world branch from 80127e8 to 38f626f Compare May 17, 2023 22:57
Copy link
Member

@alexandersandberg alexandersandberg left a comment

Choose a reason for hiding this comment

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

This will be a great addition to the Swift.org documentation! I left some smaller comments regarding the structure.

Also, we may want to mark this PR as a draft until it's ready to be merged.

documentation/index.md Show resolved Hide resolved
documentation/cxx-interop/index.md Outdated Show resolved Hide resolved
documentation/cxx-interop/index.md Outdated Show resolved Hide resolved
@hyp hyp marked this pull request as draft May 22, 2023 21:40
Copy link
Member

@alexandersandberg alexandersandberg left a comment

Choose a reason for hiding this comment

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

Great! Left two comments, at least one of which we can address later.

I will ping the rest of the SWWG as well to see if they have any feedback to give, but as everyone's quite busy these days I think it's best to just merge this when you're ready for it and then improve things if necessary in a later PR.

Edit: As I still haven't had time to look into #305, we sadly won't have time to fix the highlighting before shipping this. I will try to get it done soon though.

documentation/cxx-interop/index.md Outdated Show resolved Hide resolved
---
layout: page
title: Mix Swift and C++
official_url: https://swift.org/documentation/cxx-interop/
Copy link
Member

Choose a reason for hiding this comment

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

These official_urls should not be necessary. I'm not sure why it's used in the API Guidelines. It's something we (the SWWG) can revisit at another point though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll keep it here for now then.

Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

I've had a quick scan through and looks sensible to me other than @alexandersandberg 's comments

@dempseyatgithub
Copy link
Contributor

dempseyatgithub commented Jun 1, 2023

I had a few small comments.

Overall though this is a significant addition, thank you to everyone who wrote and contributed to this!

One additional question I did have. Will readers understand that the phrase “A development version of Swift” means an unreleased version of Swift?

Some people may read that as “A version of Swift I can use for development” and then be confused that it doesn’t work in the current release.

That phrase is used in a number of places throughout the PR.

If this support is in the development branch of Swift 5.9, maybe we should be explicit about what version it is in?

The release process for Swift 5.9 was announced back in March, so it is public info.

@tomerd
Copy link
Contributor

tomerd commented Jun 1, 2023

⚠️ please do not merge this PR @hyp will merge when all feedback is addressed ⚠️

@hyp hyp marked this pull request as draft June 1, 2023 21:39
@hyp
Copy link
Contributor Author

hyp commented Jun 2, 2023

Marked as draft for now while addressing the remaining comments today

@hyp hyp marked this pull request as ready for review June 5, 2023 20:51
@hyp
Copy link
Contributor Author

hyp commented Jun 5, 2023

@swift-ci please test

@hyp
Copy link
Contributor Author

hyp commented Jun 5, 2023

@swift-ci test

@hyp hyp merged commit e1692b5 into swiftlang:main Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants