-
Notifications
You must be signed in to change notification settings - Fork 1k
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
KTOR 1344 - Unified pre-typed routes #2211
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except for bytecode duplication
ktor-server/ktor-server-tests/jvm/test/io/ktor/tests/server/routing/RoutingProcessingTest.kt
Outdated
Show resolved
Hide resolved
ktor-server/ktor-server-tests/jvm/test/io/ktor/tests/server/routing/RoutingProcessingTest.kt
Outdated
Show resolved
Hide resolved
ktor-server/ktor-server-core/jvm/src/io/ktor/routing/RoutingBuilder.kt
Outdated
Show resolved
Hide resolved
@rsinukov This generally looks fine but I feel these function may/will clash with locations functions so I wonder if we can somehow workaround it? The functions contributed seems useful but I don't know how do we avoid ambiguity. What do you think? |
@cy6erGn0m it looks ok for me. Locations builders and these ones are in different packages, so users can always specify what function they want to use. We already have this for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please run ./gradlew apiDump
and change the target branch to 1.6.0
160f811
to
e478f90
Compare
@rsinukov Changed. apiDump did not update my changes, because all functions are |
@hfhbd Ok, let's proceed with merge. Could you please rebase it and skip irrelevant commits? Notice the base branch 1.6.0-eap |
Dont merge, one second, I want to fix my username |
Okay, I don't care about the username |
* KTOR-1344 Add failing test * KTOR-1344 Add put and post typed overload * KTOR-1344 Split typed functions into two, one with path and one without * KTOR-1344 Refactor test * KTOR-1344 Remove path from comments Co-authored-by: hfhbd <[email protected]>
* KTOR-1344 Add failing test * KTOR-1344 Add put and post typed overload * KTOR-1344 Split typed functions into two, one with path and one without * KTOR-1344 Refactor test * KTOR-1344 Remove path from comments Co-authored-by: hfhbd <[email protected]>
* KTOR-1344 Add failing test * KTOR-1344 Add put and post typed overload * KTOR-1344 Split typed functions into two, one with path and one without * KTOR-1344 Refactor test * KTOR-1344 Remove path from comments Co-authored-by: hfhbd <[email protected]>
* KTOR-1344 Add failing test * KTOR-1344 Add put and post typed overload * KTOR-1344 Split typed functions into two, one with path and one without * KTOR-1344 Refactor test * KTOR-1344 Remove path from comments Co-authored-by: hfhbd <[email protected]>
* KTOR-1344 Add failing test * KTOR-1344 Add put and post typed overload * KTOR-1344 Split typed functions into two, one with path and one without * KTOR-1344 Refactor test * KTOR-1344 Remove path from comments Co-authored-by: hfhbd <[email protected]>
* KTOR-1344 Add failing test * KTOR-1344 Add put and post typed overload * KTOR-1344 Split typed functions into two, one with path and one without * KTOR-1344 Refactor test * KTOR-1344 Remove path from comments Co-authored-by: hfhbd <[email protected]>
* KTOR-1344 Add failing test * KTOR-1344 Add put and post typed overload * KTOR-1344 Split typed functions into two, one with path and one without * KTOR-1344 Refactor test * KTOR-1344 Remove path from comments Co-authored-by: hfhbd <[email protected]>
* KTOR-1344 Add failing test * KTOR-1344 Add put and post typed overload * KTOR-1344 Split typed functions into two, one with path and one without * KTOR-1344 Refactor test * KTOR-1344 Remove path from comments Co-authored-by: hfhbd <[email protected]>
* KTOR-1344 Add failing test * KTOR-1344 Add put and post typed overload * KTOR-1344 Split typed functions into two, one with path and one without * KTOR-1344 Refactor test * KTOR-1344 Remove path from comments Co-authored-by: hfhbd <[email protected]>
* KTOR-1344 Add failing test * KTOR-1344 Add put and post typed overload * KTOR-1344 Split typed functions into two, one with path and one without * KTOR-1344 Refactor test * KTOR-1344 Remove path from comments Co-authored-by: hfhbd <[email protected]>
* KTOR-1344 Add failing test * KTOR-1344 Add put and post typed overload * KTOR-1344 Split typed functions into two, one with path and one without * KTOR-1344 Refactor test * KTOR-1344 Remove path from comments Co-authored-by: hfhbd <[email protected]>
Subsystem
Server - Routing
Motivation
Currently, only
post
has a typed overload. KTOR-1344Solution
Added a typed overload to
put
andpatch
as well