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

docs: ClientBootstrap reuse is not important but permissible on the same thread/EL #2520

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

Conversation

weissi
Copy link
Member

@weissi weissi commented Sep 21, 2023

Motivation:

The original ClientBootstrap documentation is sometimes misunderstood as stressing the importance to re-use.

Modifications:

Explain that reuse is not important but permissible on the same thread/EL.

Result:

Clearer docs.

@weissi weissi changed the title Update Bootstrap.swift docs: ClientBootstrap reuse is not important but permissible on the same thread/EL Sep 21, 2023
Sources/NIOPosix/Bootstrap.swift Outdated Show resolved Hide resolved
Sources/NIOPosix/Bootstrap.swift Outdated Show resolved Hide resolved
@tayloraswift I needed to undo the ``Sendable`` and ``EventLoop`` because they're in different modules :|
@weissi weissi enabled auto-merge (squash) September 22, 2023 14:42
@tayloraswift
Copy link
Contributor

it would be really great if we could suppress the DocC symbol link warnings, those cross-module/cross-package links work on Swiftinit!

@weissi
Copy link
Member Author

weissi commented Sep 22, 2023

it would be really great if we could suppress the DocC symbol link warnings, those cross-module/cross-package links work on Swiftinit!

@franklinsch is this an option? Or will this work in DocC soon too?

Context is that I had to take out

``Sendable``

and

``EventLoop``

because it complained

/// Usually you re-use a `ClientBootstrap` once you set it up and called `connect` multiple times on it.
/// This way you ensure that the same `EventLoop`s will be shared across all your connections.
/// You may re-use a `ClientBootstrap` once you set it up and call `connect` multiple times on it.
/// This ensures all connections you create share the same `EventLoop`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not true. We are taking the next EventLoop from the EventLoopGroup in connect.

Comment on lines +720 to +725
/// Creating a `ClientBootstrap` is cheap. So instead of arranging synchronization allowing
/// concurrent code to re-use a single `ClientBootstrap` instance across many tasks, it is often
/// easier to create a dedicated instance for each task.
/// multiple threads/`EventLoop`s. Creating a `ClientBootstrap` is cheap so instead of arranging
/// synchronization to re-use a single `ClientBootstrap` instance across threads, it's advisable to
/// create fresh instances.
Copy link
Contributor

Choose a reason for hiding this comment

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

This here is somewhat duplicated

@franklinsch
Copy link
Member

@weissi We're tracking cross-target link resolution with apple/swift-docc#208. It's being worked on primarily by @d-ronnqvist

@d-ronnqvist
Copy link

d-ronnqvist commented Oct 25, 2023

@weissi We're tracking cross-target link resolution with apple/swift-docc#208. It's being worked on primarily by @d-ronnqvist

The initial support for cross-target linking in DocC is available on main behind a feature flag but the overall feature of multi-target documentation is still in progress.

Note that the planned—and currently implemented—syntax for linking to a symbol in another module is

``/OtherModuleName/SymbolName/functionName()``

stating with a leading / to indicate an external symbol link, followed by the absolute symbol path (including the name of the external module). For example:

  • /Swift/Sendable
  • /NIOCore/EventLoop

@weissi
Copy link
Member Author

weissi commented Jan 3, 2024

@weissi We're tracking cross-target link resolution with apple/swift-docc#208. It's being worked on primarily by @d-ronnqvist

The initial support for cross-target linking in DocC is available on main behind a feature flag but the overall feature of multi-target documentation is still in progress.

Note that the planned—and currently implemented—syntax for linking to a symbol in another module is

``/OtherModuleName/SymbolName/functionName()``

stating with a leading / to indicate an external symbol link, followed by the absolute symbol path (including the name of the external module). For example:

  • /Swift/Sendable
  • /NIOCore/EventLoop

@dnadoba I assume NIO isn't using feature flags for its docs right now so this has to stay as is, correct?

@dnadoba
Copy link
Member

dnadoba commented Jan 8, 2024

I'm not aware of any feature flags set for DocC in NIO. The two main way DocC is compiled is through the build system provided by https://swiftpackageindex.com/ and Xcodes "Product -> Build Documentation" action. I'm not aware of a safe way to add flags to both of them.

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.

None yet

6 participants