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

collection of renames for 2.0 #663

Closed
1 task
weissi opened this issue Nov 27, 2018 · 14 comments
Closed
1 task

collection of renames for 2.0 #663

weissi opened this issue Nov 27, 2018 · 14 comments
Labels
kind/administration CI, repo automation, other admin tasks.
Milestone

Comments

@weissi
Copy link
Member

weissi commented Nov 27, 2018

This is supposed to become a crowd-sourced random collection of renames in SwiftNIO for the 2.0 release. Please just add something like

  • rename FooBar(bar: Bar) to FooBar(_ bar: Bar)
@weissi weissi added this to the 2.0.0 milestone Nov 27, 2018
@weissi
Copy link
Member Author

weissi commented Nov 27, 2018

  • change ChannelPipeline.add(name: String? = nil, handler: ChannelHandler, first: Bool = false) to ChannelPipeline.addHandler(_ handler: ChannelHandler, name: String?, first: Bool = false)
  • change all ports to Int instead of UInt16, right now we're inconsistent
  • change ByteBuffer.set(xxx: ...) to ByteBuffer.setXXX(_ xxx: ...), same for write(xxx:
  • rename Thread to NIOThread (and more Foundation collisions)
  • rename FileHandle to NIOFileHandle (and more Foundation collisions)
  • rename SSLContext to NIOSSLContext (and more Foundation collisions)
  • go hunt and remove public extensions of stdlib things that don't use NIO types
  • (remove ELP.succeed/fail's labels #776) remove all the labels involved in creating successful/failed futures (such as makeSucceededFuture(result:), the result: part only adds noise

@weissi
Copy link
Member Author

weissi commented Nov 27, 2018

again, CC'ing some folks @helje5 / @tanner0101 / @ianpartridge / @pushkarnk / @MrMage / @tachyonics / @airspeedswift / @moiseev / @kjessup / @normanmaurer / @Lukasa / @Joannis . Please name your small pet peeves here :)

@MrMage
Copy link
Contributor

MrMage commented Nov 27, 2018

  • rename ctx to context

What’s the motivation for changing port numbers to Int? Aren’t they always unsigned 16-bit integers?

@weissi
Copy link
Member Author

weissi commented Nov 27, 2018

  • rename ctx to context

What’s the motivation for changing port numbers to Int? Aren’t they always unsigned 16-bit integers?

it's Swift convention to always use Int for everything (at least in public API unless it really doesn't work, like nanoseconds timeouts need to be Int64 because 32 bit platforms) because it makes interoperability work much better. See for example Array's var count: Int even though clearly it's never negative...

@MrMage
Copy link
Contributor

MrMage commented Nov 27, 2018 via email

@weissi weissi added the task label Nov 28, 2018
@weissi
Copy link
Member Author

weissi commented Nov 28, 2018

  • rename new*Promise to make*Promise as of the Swift API guidelines: "Begin names of factory methods with “make”, e.g. x.makeIterator()"
  • rename newAddressResolving(host: String, port: Int) to makeAddress(resolvingHost: String, port: Int)
  • rename ChannelCore to _ChannelCore as it's public but not part of the public API (as documented)
  • rename Sni* to SNI* like SNIHandler or SNIResult

@ianpartridge
Copy link
Contributor

Do you want to tackle #602 in 2.0 too?

@weissi
Copy link
Member Author

weissi commented Nov 28, 2018

@ianpartridge absolutely! thanks for reminding me

@weissi weissi changed the title collection of small random renames for 2.0 collection of renames for 2.0 Jan 19, 2019
@weissi
Copy link
Member Author

weissi commented Jan 19, 2019

  • rename then to flatMap
  • rename thenIfError to flatMapError

to match Result in the stdlib

@tanner0101
Copy link
Contributor

  • rename thenThrowing to mapThrowing and add a thenThrowing that accepts a future.
  • make pipeline handler add / remove methods more consistent. i.e., all should support before / after, first, array, name, etc.
  • add promise.succeed() with no args where Success == Void
  • add convenience method for creating required wrapper to ByteToMessageDecoder. (maybe PostgresMessageDecoder().handler?)
  • add cascadeSuccess to future.
    (I'll add to this list if I think of any more)

@Lukasa
Copy link
Contributor

Lukasa commented Jan 23, 2019

What does remove(before:) mean? I don’t think that one makes much sense.

@tanner0101
Copy link
Contributor

tanner0101 commented Jan 23, 2019

What does remove(before:) mean? I don’t think that one makes much sense.

Sorry, the comment should just say "add methods".

screen shot 2019-01-23 at 5 02 42 pm

Mostly I just think it would be nice to have the option to specify name / after / before when adding multiple handlers. For name, something like [String: ChannelHandler] would be nice. I also think naming the multiple add method add(handlers:) might be more Swifty.

@weissi
Copy link
Member Author

weissi commented Jan 24, 2019

  • rename thenThrowing to mapThrowing and add a thenThrowing that accepts a future.

strong -1 to any name that starts with map*, that name would be just wrong. map's signature is

(ELF<Value>) -> ((Value -> NewValue)) -> ELF<NewValue>

, the important part here is that you can only transform a success to a success and nothing else.

flatMapThrowing (used to be thenThrowing) is a good name because its name is exactly what it does: It takes a throwing thing, converts that to an ELF and then flatMaps that. Works. You just can't implement that function with a map only but you can with a flatMap.

Namely, there's a natural translation from

(A) throws -> B

to

(A) -> EventLoopFuture<B>

which is exactly the right input type for a flatMap. But there isn't any from

(A) throws -> B

to

(A) -> B

which would be the right input type for a map. I'm open to giving it some other name combineThrowing but it can't start with map because that's really wrong and misleading. My point is still that flatMapThrowing is arguably the best name because that's what it is (see it's implementation):

    public func flatMapThrowing<NewValue>(file: StaticString = #file,
                                line: UInt = #line,
                                _ callback: @escaping (Value) throws -> NewValue) -> EventLoopFuture<NewValue> {
        return self.flatMap(file: file, line: line) { (value: Value) -> EventLoopFuture<NewValue> in
            do {
                return EventLoopFuture<NewValue>(eventLoop: self.eventLoop, result: try callback(value), file: file, line: line)
            } catch {
                return EventLoopFuture<NewValue>(eventLoop: self.eventLoop, error: error, file: file, line: line)
            }
        }
    }

but I realise that people don't like it so I'd be open to combineThrowing or so.

  • add promise.succeed() with no args where Success == Void

-1 to this one too. Overloads always lead to sadness and this one saves two characters (#776 is the PR to get rid of the labels for succeed and fail).

  • add cascadeSuccess to future.

+1

@weissi
Copy link
Member Author

weissi commented Feb 25, 2019

closing because mostly done apart from the work already filed as #483

@weissi weissi closed this as completed Feb 25, 2019
@FranzBusch FranzBusch added the kind/administration CI, repo automation, other admin tasks. label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/administration CI, repo automation, other admin tasks.
Projects
None yet
Development

No branches or pull requests

6 participants