Skip to content

feat: Add unchecked variants to NIOLoopBound(Box) - #3678

Open
wasim-builds wants to merge 4 commits into
apple:mainfrom
wasim-builds:feature-nioloopbound-unchecked
Open

feat: Add unchecked variants to NIOLoopBound(Box)#3678
wasim-builds wants to merge 4 commits into
apple:mainfrom
wasim-builds:feature-nioloopbound-unchecked

Conversation

@wasim-builds

Copy link
Copy Markdown
Contributor

Fixes #2506

Comment thread Sources/NIOCore/NIOLoopBound.swift Outdated
///
/// - Note: This is an unchecked variant of ``init(_:eventLoop:)`` that only asserts in debug mode.
@inlinable
public init(unchecked value: Value, eventLoop: EventLoop) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's make these names a bit more explicit about what we're not checking, e.g.

Suggested change
public init(unchecked value: Value, eventLoop: EventLoop) {
public init(uncheckedOnEventLoop value: Value, eventLoop: EventLoop) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I might also suggest we call these uncheckedUnsafe to clarify what's happening.

@wasim-builds

Copy link
Copy Markdown
Contributor Author

Thanks @Lukasa for the review! I've updated the initializer parameter label and properties to use uncheckedUnsafe as suggested.

Address review feedback from Lukasa: the init parameter should be
'uncheckedOnEventLoop' to clarify what specifically is unchecked
(the event loop assertion), not 'uncheckedUnsafe' which better
describes the property/method accessors.
@wasim-builds

Copy link
Copy Markdown
Contributor Author

Thanks @Lukasa! Applied both pieces of feedback:

  1. Renamed init parameter to uncheckedOnEventLoop to clarify what specifically is unchecked (the event loop assertion)
  2. Kept uncheckedUnsafe prefix on properties/methods as you suggested

Branch has been updated.

@wasim-builds

Copy link
Copy Markdown
Contributor Author

By the way, I've really been enjoying contributing to this project! My goal is to get more involved in open source to help maintain projects and learn from great engineering teams. Is there a process or pathway for becoming a member of the apple GitHub organization? I'd love to continue helping out where I can!

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.

NIOLoopBound should get unchecked variants that are truly free in release mode

2 participants