Skip to content

Conversation

@Nao-RandD
Copy link
Contributor

@Nao-RandD Nao-RandD commented Mar 24, 2024

Motivation

  • We want to avoid redundant code by removing unnecessary self keywords

Modifications

  • Removed unnecessary self keywords

Result

  • No errors, ./scripts/soundness.sh also passed.

Test Plan

[Describe the steps you took, or will take, to qualify the change - such as adjusting tests and manual testing.]

@Nao-RandD Nao-RandD marked this pull request as ready for review March 24, 2024 10:22
@czechboy0
Copy link
Contributor

@swift-server-bot test this please

@czechboy0
Copy link
Contributor

@simonjbeaumont

@simonjbeaumont
Copy link
Collaborator

First off: sorry for the time it took to notice the PR; I've been OOO recently.

Thanks for taking the time here and for caring about consistent style :)

I'm in principle happy for us to adopt this more consistently—we have a similar style in the other swift-openapi-* repos—but I'd like to preserve the contents of Sources/BufferedStream/* as-is, because they have been vendored in from another project and we explicitly ignore any of the swift-format rules for these files1.

Happy to take the rest of the PR though, and I'm happy to update it, to save you the work.

Footnotes

  1. https://github.com/apple/swift-openapi-urlsession/blob/f81270ea80d9fba9d33f2ac147a79765302124c0/Sources/OpenAPIURLSession/BufferedStream/BufferedStream.swift#L14

@Nao-RandD
Copy link
Contributor Author

@simonjbeaumont
Thanks for confirming this.

I understand that you want to keep the contents of Sources/BufferedStream/* intact.

I have restored the relevant modifications with 1134386 , so please check again.

@simonjbeaumont
Copy link
Collaborator

@swift-server-bot test this please

@simonjbeaumont simonjbeaumont enabled auto-merge (squash) April 16, 2024 14:20
@simonjbeaumont simonjbeaumont merged commit 6b915d8 into apple:main Apr 16, 2024
@Nao-RandD Nao-RandD deleted the sb/remove-unnecessary-self branch April 21, 2024 07:16
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants