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

Update Scala Native API and worker #2193

Merged
merged 1 commit into from
Dec 14, 2022
Merged

Conversation

lolgab
Copy link
Member

@lolgab lolgab commented Dec 12, 2022

Drop support for Scala Native 0.4.0 and 0.4.1

@lefou
Copy link
Member

lefou commented Dec 13, 2022

I'm still struggeling with the api/worker-api and worker naming. First, it's not really a typical API, rather some subset SPI to share some classes to define a worker implementation.

In some of my plugin projects I switched to worker (for the former worker api) and worker.impl or worker.<better-name-for-the-concrete-worker-impl>, and find it nicer.

WDYT?

Drop support for Scala Native 0.4.0 and 0.4.1
@lolgab
Copy link
Member Author

lolgab commented Dec 13, 2022

Can I do that in a separate PR where I use the same convention for all the workers? I just copied scalajslib here.
I don't dislike the api name, since that package has only traits and classes used in the worker implementation. But I don't know what is the best naming convention honestly.

@lolgab lolgab marked this pull request as ready for review December 13, 2022 11:56
@lolgab lolgab requested a review from lefou December 13, 2022 11:56
Copy link
Member

@lefou lefou left a comment

Choose a reason for hiding this comment

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

Looks good to me. I'm not that much a fan of multiple classes in the same source file. Also the limited scope of the worker api trait is IMHO not necessary and may stand in the way for some individual project customizations. Alternatively we could annotate it @internal. But do as you think is best, it's your maintenance job. We probably should review the published version policy for worker modules before we make the next stable release.

@lolgab lolgab merged commit 74e0d46 into com-lihaoyi:main Dec 14, 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.

None yet

2 participants