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 to a protocol-based interface for core #275

Open
cmyr opened this Issue Sep 27, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@cmyr
Member

cmyr commented Sep 27, 2018

A refactor that's long overdue: just as we define a client interface in Client.swift, we should define a core interface. All the various core methods would be presented through this interface; behind the surface it would wrap something like the existing CoreConnection class.

This will make it easy for us to change how the core connection is managed at some point in the future, and will also let us write tests in xi-mac that don't need to spin up a real instance of the core.

@jkaan

This comment has been minimized.

Show comment
Hide comment
@jkaan

jkaan Oct 2, 2018

@cmyr Hi there! I could pick this one up. Would it for now just involve putting the public interface of the CoreConnection into a protocol and typehinting to that interface instead of the concrete implementation?

jkaan commented Oct 2, 2018

@cmyr Hi there! I could pick this one up. Would it for now just involve putting the public interface of the CoreConnection into a protocol and typehinting to that interface instead of the concrete implementation?

@cmyr

This comment has been minimized.

Show comment
Hide comment
@cmyr

cmyr Oct 2, 2018

Member

CoreConnection doesn't really have a typed interface at all; it just exposes functions for sending raw arbitrary RPCs. What we'd really like is to have a protocol that describes the legal, existing RPCs, and then implements that for CoreConnection by translating them into the raw RPCs.

We might also consider having a protocol for the CoreView interface, which describes just the RPCs that are scoped to a view.

There is a lot of RPC surface area (especially considering all of the movement commands) so I think it would be okay to go halfway, initially; for instance you could have a protocol method "raw_notification" that still lets you specify a command by it's (string) method name.

edit: hello! 👋

Member

cmyr commented Oct 2, 2018

CoreConnection doesn't really have a typed interface at all; it just exposes functions for sending raw arbitrary RPCs. What we'd really like is to have a protocol that describes the legal, existing RPCs, and then implements that for CoreConnection by translating them into the raw RPCs.

We might also consider having a protocol for the CoreView interface, which describes just the RPCs that are scoped to a view.

There is a lot of RPC surface area (especially considering all of the movement commands) so I think it would be okay to go halfway, initially; for instance you could have a protocol method "raw_notification" that still lets you specify a command by it's (string) method name.

edit: hello! 👋

@cmyr

This comment has been minimized.

Show comment
Hide comment
@cmyr

cmyr Oct 2, 2018

Member

oh, also: there was an earlier attempt at something like this, in Dispatcher.swift. This predates my involvement in the project, and doesn't seem to have really ever been consistently used; I would expect this new work to also result in us removing that file, I think?

Member

cmyr commented Oct 2, 2018

oh, also: there was an earlier attempt at something like this, in Dispatcher.swift. This predates my involvement in the project, and doesn't seem to have really ever been consistently used; I would expect this new work to also result in us removing that file, I think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment