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

Add documentation comments and a defaulting subscript to Storage. #2857

Merged
merged 3 commits into from Jul 21, 2022

Conversation

gwynne
Copy link
Member

@gwynne gwynne commented Jul 15, 2022

The new subscript simplifies "provider" implementations that extend Application and use its Storage instance without complex initialization requirements:

extension Application {
    public struct Foo {
        final class Storage { /* content which needs no special initialization */ }
        struct Key: StorageKey { typealias Value = Storage }
        let application: Application

        // Before:
        var storage: Storage {
            if self.application.storage[Key.self] == nil { self.initialize() }
            return self.application.storage[Key.self]!
        }

        func initialize() { self.application.storage[Key.self] = .init() }

        // After:
        var storage: Storage { self.application.storage[Key.self, default: .init()] }

@gwynne gwynne added enhancement New feature or request semver-minor Contains new API labels Jul 15, 2022
@gwynne gwynne requested review from vzsg, 0xTim and jdmcd July 15, 2022 21:16
@gwynne gwynne added this to Awaiting Review in Vapor 4 via automation Jul 15, 2022
@gwynne gwynne self-assigned this Jul 15, 2022
Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

LGTM, one potential clean up

Sources/Vapor/Utilities/Storage.swift Outdated Show resolved Hide resolved
Copy link
Member

@vzsg vzsg left a comment

Choose a reason for hiding this comment

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

I agree with Tim about removing that force unwrap, otherwise LGTM 👍🏻

Sources/Vapor/Utilities/Storage.swift Outdated Show resolved Hide resolved
@gwynne gwynne force-pushed the storage-defaulting-subscript branch from 3761130 to c226a66 Compare July 21, 2022 06:01
gwynne and others added 2 commits July 21, 2022 01:03
Co-authored-by: Tim Condon <0xTim@users.noreply.github.com>
@gwynne gwynne merged commit d9d83cd into main Jul 21, 2022
Vapor 4 automation moved this from Awaiting Review to Done Jul 21, 2022
@gwynne gwynne deleted the storage-defaulting-subscript branch July 21, 2022 06:41
@VaporBot
Copy link
Contributor

These changes are now available in 4.63.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request semver-minor Contains new API
Projects
Vapor 4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants