-
Notifications
You must be signed in to change notification settings - Fork 7
Make SubmissionsMiddleware ServiceType compliant to add middleware in config as the rest of vapor's available middleware (coding style more than a real improvement) #49
base: master
Are you sure you want to change the base?
Conversation
siemensikkema
left a comment
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.
Much better, thanks 🙂 Just a couple of nitpicks, but otherwise 👍
| public final class SubmissionsMiddleware: Middleware { | ||
| public final class SubmissionsMiddleware: Middleware, ServiceType { | ||
|
|
||
| /// See `ServiceType`. |
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 use spaces here instead of tabs :)
README.md
Outdated
| try services.register(SubmissionsProvider()) | ||
| ``` | ||
|
|
||
| It also include a middleware you have to register in your `MiddlewareConfig` : |
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.
*includes
Allow vapor to find it as a Middleware
|
I took care to correct from your review and improve a little bit the doc. Also add a commit of code (Wasn't sufficient for vapor to make Middleware ServiceType compliant) should really work now. |
|
|
||
| /// See `ServiceType`. | ||
| /// See `ServiceType`. | ||
| public static func makeService(for container: Container) throws -> SubmissionsMiddleware { |
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.
These are tabs, please use spaces 🙂 (in the whole function)
|
|
||
| /// See `Provider` | ||
| public func register(_ services: inout Services) throws { | ||
| services.register(SubmissionsMiddleware.self) |
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.
more tabs that should be spaces 🙂
Self-describing (and from #48)