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

Move Op from parameter to member in smithy4s.Service #567

Merged
merged 22 commits into from
Nov 9, 2022

Conversation

Baccata
Copy link
Contributor

@Baccata Baccata commented Nov 2, 2022

Similarly to what happened with cats.Parallel, the Operation type of smithy4s.Service can be moved from type-parameter to type-member. This simplifies type signatures for a number of public methods, and helps implicit search in some cases.

Similarly to what happened with `cats.Parallel`, the Operation type of
`smithy4s.Service` can be moved from type-parameter to type-member,
which helps implicit search.
implicit val serviceInstance: Service[Alg, Op] = this
val service = this
trait Service[Alg[_[_, _, _, _, _]]] extends FunctorK5[Alg] with Service.Provider[Alg] {
type Operation[E, I, O, SI, SO]
Copy link
Contributor

Choose a reason for hiding this comment

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

allows for a long name :D (not that it was not allowed of course, just that the long version in the type parameter as just so annoying

@kubukoz
Copy link
Member

kubukoz commented Nov 2, 2022

sooo why didn't CI fail on any of the last commits?

@lewisjkl
Copy link
Contributor

lewisjkl commented Nov 2, 2022

sooo why didn't CI fail on any of the last commits?

The CI, as currently configured, doesn't run unless the PR is targeting main (afaik)

@kubukoz
Copy link
Member

kubukoz commented Nov 2, 2022

we should probably change that, then :)

@Baccata Baccata marked this pull request as ready for review November 3, 2022 11:33
@Baccata
Copy link
Contributor Author

Baccata commented Nov 3, 2022

Problem was that PR trigger was using the * glob pattern when it should be using the ** (there might be slashes in the branch name)

trait Service[Alg[_[_, _, _, _, _]], Op[_, _, _, _, _]] extends FunctorK5[Alg] with Service.Provider[Alg, Op] {
implicit val serviceInstance: Service[Alg, Op] = this
val service = this
trait Service[Alg[_[_, _, _, _, _]]] extends FunctorK5[Alg] with Service.Provider[Alg] {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of the changes derive from this line : the Op is moved from type-parameter to type-member.

type Endpoint[I, E, O, SI, SO] = smithy4s.Endpoint[Operation, I, E, O, SI, SO]
type Interpreter[F[_, _, _, _, _]] = PolyFunction5[Operation, F]
type FunctorInterpreter[F[_]] = PolyFunction5[Operation, kinds.Kind1[F]#toKind5]
type BiFunctorInterpreter[F[_, _]] = PolyFunction5[Operation, kinds.Kind2[F]#toKind5]
Copy link
Contributor Author

@Baccata Baccata Nov 3, 2022

Choose a reason for hiding this comment

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

the type aliases above are path-dependant (by virtue of depending on the Operation abstract type member) , it means we can ascribe service.Endpoint, service.Interpreter, etc etc. This allows to simplify up a number of verbose ascriptions here and there.

service,
service.toPolyFunction[Kind1[F]#toKind5](impl),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the implementation was prematurely turned into a polymorphic function


def apply[Alg[_[_, _, _, _, _]]](implicit ev: Service[Alg]): ev.type = ev

type Aux[Alg[_[_, _, _, _, _]], Op[_, _, _, _, _]] = Service[Alg]{ type Operation[I, E, O, SI, SO] = Op[I, E, O, SI, SO] }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the Aux pattern allows to "recover" the type parameter that was moved, in locations where it matters

@Baccata Baccata merged commit 4006e3d into series/0.17 Nov 9, 2022
@Baccata Baccata deleted the change-service-encoding branch November 9, 2022 11:12
@Baccata Baccata added this to the 0.17.0 milestone Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants