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 convenience protocol for enums #112

Merged
merged 3 commits into from
May 16, 2018
Merged

Add convenience protocol for enums #112

merged 3 commits into from
May 16, 2018

Conversation

tholo
Copy link
Contributor

@tholo tholo commented May 16, 2018

Allow enums to be easily saved and restored from native types,
modeled on implementation from SQLite.

Allow enums to be easily saved and restored from native types,
modeled on implementation from SQLite.

/// See `MySQLDataConvertible.convertFromMySQLData(_:)`
public static func convertFromMySQLData(_ data: MySQLData) throws -> Self {
guard let e = try self.init(rawValue: .convertFromMySQLData(data)) else {
Copy link
Member

@calebkleveter calebkleveter May 16, 2018

Choose a reason for hiding this comment

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

This constant should have a different name. Maybe instance or extractedCase? Just something that is more descriptive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems reasonable. Someone should likely go back and do that in the SQLite enum code where this was stol... err... got inspiration from?

/// Provides a default `MySQLDataConvertible` implementation where the type is also
/// `RawRepresentable` by a `MySQLDataConvertible` type.
extension MySQLDataConvertible
where Self: RawRepresentable, Self.RawValue: MySQLDataConvertible
Copy link
Member

Choose a reason for hiding this comment

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

The line break here should probably be removed. It doesn't match the style of the other declarations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah -- I think that was a left-over due to the original (SQLite) variant had some much longer things and "had" to be on two lines. But that is no longer the case here...

tholo added 2 commits May 16, 2018 11:17
This is not long enough to require a line break, unlike the
original it was taken from.
Use a descriptive name for an assigned constant instead of a single
letter name.
Copy link
Member

@tanner0101 tanner0101 left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@tanner0101 tanner0101 added the enhancement New feature or request label May 16, 2018
@tanner0101 tanner0101 self-assigned this May 16, 2018
@tanner0101 tanner0101 added this to the 3.0.0-rc.2.5 milestone May 16, 2018
@tanner0101
Copy link
Member

Fixes #103 and #99

@tanner0101
Copy link
Member

@calebkleveter do you approve of the changes now?

@penny-coin
Copy link

Hey @tholo, you just merged a pull request, have a coin!

You now have 1 coins.

@tanner0101 tanner0101 merged commit 258215e into vapor:master May 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Vapor 3
  
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

4 participants