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

codegen: do not extend from service for powerapis #1209

Merged
merged 1 commit into from Nov 24, 2020

Conversation

jrudolph
Copy link
Member

Having those extends service just to throw an exception seems like a potential trap. I haven't followed previous discussions about that, so maybe there was a good reason that things were done like that? I cannot see though how that would ever be useful the way it is right now.

There might be compatibility reasons not to change it right now.

Copy link
Member

@ignasi35 ignasi35 left a comment

Choose a reason for hiding this comment

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

Yep. Looks like a left-over to me. (Unless there's some hidden cast somewhere)

Copy link
Member

@raboof raboof left a comment

Choose a reason for hiding this comment

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

Hmm, this could allow creating one API that 'transparently' takes any service implementation (power or not).

Let's indeed remove this now, and re-introduce it if we ever encounter a strong need for such an API.

@raboof raboof merged commit 6d7ef62 into master Nov 24, 2020
@jrudolph jrudolph deleted the remove-extends-when-using-powerapi branch July 15, 2021 15:06
@raboof
Copy link
Member

raboof commented Nov 8, 2021

I think I remember the reason: when navigating a codebase, it is kind of nice to be able to go to the service interface, ask your IDE to find all implementations, and get both the 'simple' and power-api implementations.

I don't think that's reason enough to bring it back, though.

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.

None yet

3 participants