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 Service.name() to misk module #1214

Conversation

@jjestrel
Copy link
Collaborator

commented Oct 2, 2019

No description provided.

Jay Estrella

import com.google.common.util.concurrent.Service

fun Service.name(): String = when (this) {

This comment has been minimized.

Copy link
@jjestrel

jjestrel Oct 2, 2019

Author Collaborator

I'm open to putting this in a different package. This way we have misk expose a way to get a "name" then callers don't need to care (at the call site) about dealing with CoordinatedService vs plain service

This comment has been minimized.

Copy link
@swankjesse

swankjesse Oct 2, 2019

Collaborator

I think this wants to be CoordinatedService’s toString().

This comment has been minimized.

Copy link
@adrw

adrw Oct 2, 2019

Collaborator

You could move this and all related Service files (CoordinatorService, MiskServiceModule, ServiceGraphBuilder, ServiceModule...) into a misk.service package.

This comment has been minimized.

Copy link
@jjestrel

jjestrel Oct 2, 2019

Author Collaborator

I think the toString change is a different unit of work than this PR. It would be nice if Service just had name() in the interface

This comment has been minimized.

Copy link
@swankjesse

swankjesse Oct 2, 2019

Collaborator

Unfortunately that’s not how extension methods work. In particular, custom implementations of Service cannot override this method to provide their own name.

@jjestrel jjestrel requested review from keeferrourke and swankjesse Oct 2, 2019
@adrw
adrw approved these changes Oct 2, 2019

import com.google.common.util.concurrent.Service

fun Service.name(): String = when (this) {

This comment has been minimized.

Copy link
@adrw

adrw Oct 2, 2019

Collaborator

You could move this and all related Service files (CoordinatorService, MiskServiceModule, ServiceGraphBuilder, ServiceModule...) into a misk.service package.

@jjestrel jjestrel closed this Oct 13, 2019
@jjestrel

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 13, 2019

We did something else

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.