Skip to content
This repository was archived by the owner on Apr 20, 2024. It is now read-only.

Conversation

@steffendsommer
Copy link
Contributor

@steffendsommer steffendsommer commented Oct 29, 2018

@codecov
Copy link

codecov bot commented Oct 29, 2018

Codecov Report

Merging #122 into vapor-3 will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           vapor-3   #122   +/-   ##
======================================
  Coverage        0%     0%           
======================================
  Files           24     25    +1     
  Lines          945    937    -8     
======================================
+ Misses         945    937    -8
Impacted Files Coverage Δ
Sources/AdminPanel/Tags/AdminPanelConfigTag.swift 0% <ø> (ø) ⬆️
Sources/AdminPanel/Models/AdminPanelUserType.swift 0% <ø> (ø) ⬆️
.../AdminPanel/Commands/AdminPanelUser+Seedable.swift 0% <0%> (ø) ⬆️
Sources/AdminPanel/Configs/AdminPanelConfig.swift 0% <0%> (ø) ⬆️
...urces/AdminPanel/Controllers/ResetController.swift 0% <0%> (ø)
...nel/Models/AdminPanelUser+AdminPanelUserType.swift 0% <0%> (ø) ⬆️
...nel/Models/AdminPanelUser+PasswordResettable.swift 0% <0%> (ø) ⬆️
...urces/AdminPanel/Controllers/LoginController.swift 0% <0%> (ø) ⬆️
Sources/AdminPanel/Tags/HasRequiredRole.swift 0% <0%> (ø) ⬆️
Sources/AdminPanel/routes.swift 0% <0%> (ø) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be6b9ef...58780d6. Read the comment docs.

@steffendsommer steffendsommer changed the title WIP: Feature/misc fixes Feature/misc fixes Nov 8, 2018
Copy link
Contributor

@siemensikkema siemensikkema left a comment

Choose a reason for hiding this comment

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

Looks good! Some minor comments ..

let config: AdminPanelConfig<U> = try req.make()
return U.query(on: req).all()
.flatMap(to: Response.self) { users in
let paginator: Future<OffsetPaginator<U>> = try U.query(on: req).paginate(for: req)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it still necessary to specify the explicit type here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm afraid so since the return type is Response.

internal func resetPasswordRequest(_ req: Request) throws -> Future<Response> {
let resetConfig: ResetConfig<U> = try req.make()
let adminPanelConfig: AdminPanelConfig<U> = try req.make()
let submission = try req.content.decode(U.RequestReset.Submission.self)
Copy link
Contributor

Choose a reason for hiding this comment

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

Guidelines ... ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm afraid I cannot do this here. The signature looks like this:

public func decode<D>(_ content: D.Type, maxSize: Int = 65_536) throws -> Future<D> where D: Decodable

and there's no overload with a default value on content.

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh yeah that's a no-go then. Would be nice to detect all these occurrences in Vapor's code and fix them.

import Flash
import Fluent
import Leaf
import Paginator
Copy link
Contributor

Choose a reason for hiding this comment

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

alphabetized! 😍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants