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

Convenient initialisers from ByteBuffer #1457

Merged
merged 14 commits into from
Mar 31, 2020
Merged

Convenient initialisers from ByteBuffer #1457

merged 14 commits into from
Mar 31, 2020

Conversation

Davidde94
Copy link
Contributor

Added simple initialisers to various types that consume a ByteBuffer.

Motivation:

Common operations to read a ByteBuffer as some different type T were a little too verbose.

Modifications:

New initialisers for:

  • Data
  • String
  • [UInt8]

Result:

var buffer = ...
let string = String(from: &buffer)
let data = Data(from: &buffer)
let array = Array<UInt8>(from: &buffer)

@Davidde94 Davidde94 added the 🆕 semver/minor Adds new public API. label Mar 20, 2020
@Davidde94 Davidde94 added this to the 2.16.0 milestone Mar 20, 2020
@Davidde94 Davidde94 linked an issue Mar 20, 2020 that may be closed by this pull request
4 tasks
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

I think we don't need the separate files for each of these: they can all go in ByteBuffer-aux except for the Data one which can just go next to readData.

Copy link
Member

@weissi weissi 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! This looks very useful, left a few comments

@Davidde94 Davidde94 requested review from weissi and Lukasa March 20, 2020 15:24
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Teeny tiny notes here, this patch looks great otherwise.

Davidde94 and others added 3 commits March 30, 2020 18:26
Co-Authored-By: Cory Benfield <lukasa@apple.com>
…e94/swift-nio into de/convenient-buffer-initialisers
@Davidde94 Davidde94 requested review from glbrntt and Lukasa March 30, 2020 17:30
Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

Awesome, that looks great!

@weissi weissi requested a review from tomerd March 30, 2020 21:47
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@Lukasa Lukasa merged commit 6b0ed24 into apple:master Mar 31, 2020
pull bot pushed a commit to scope-demo/swift-nio that referenced this pull request Mar 31, 2020
@Davidde94 Davidde94 deleted the de/convenient-buffer-initialisers branch March 31, 2020 09:46
@ktoso
Copy link
Member

ktoso commented Apr 2, 2020

Whoop, missed review here -- sorry fixed my email settings / filters a bit recently :-)

LGTM, really nice to see this one

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add convenient Type(from buffer: ByteBuffer) initialisers
6 participants