Skip to content

Clean up and DRY IOApp implementations #4361

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

Draft
wants to merge 5 commits into
base: series/3.x
Choose a base branch
from

Conversation

kapunga
Copy link
Contributor

@kapunga kapunga commented Apr 6, 2025

After taking a few initial passes through the various platform IOApp implementations, I've come up with an approach I am going to take. I plan on adding two layers of traits for a total of three new traits. Here is a rough diagram.

IOApp Decomposition

  • IOAppCommon - This will contain method signatures common to all implementations, but with different implementation.
  • IOAppMultiThreaded - This will contain method signatures common between JVM and Native, as they have a lot of extra bells and whistles not present in the SJS implementation.
  • IOAppPlatform - This will contain platform specific method definitions, there will be three different versions of these, one per platform.

I'm attempting to move things around over the course of the PR to the point where all three current IOApp implementations will be identical, at which point create a single definition in shared. A few goals I have in mind as I refactor things:

  • De-duplicate methods or add a method signature on a lower level trait so that documentation won't be duplicated.
  • Clean up code organization in IOApp to make it more approachable to people trying to understand what's going on in IOApp.
  • Hide methods people are better off not over-riding (such as pollingSystem or computeWorkerThreadCount) farther down the type hierarchy so library adopters will be less likely to notice / play around with them. Probably not possible due to bin-compat issues.

This PR will resolve Issue 4333

@kapunga kapunga force-pushed the issue-4333-ioapp-dry branch from 930e965 to eecbd29 Compare April 6, 2025 13:50
@djspiewak
Copy link
Member

Btw just a quick driveby to say that I think this is the right approach! I think the scaladoc is going to be a bit tricky to resolve, but maybe we can relax our requirements on that one. Also I'd like to ensure that these supertraits don't leak into the public API; ideally users should only be aware of IOApp itself. Should all be doable, just a bit of bincompat needle-threading.

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.

Find a way to DRY up the IOApps
2 participants