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 AutoService #1824

Merged
merged 3 commits into from
Jan 13, 2022
Merged

Add AutoService #1824

merged 3 commits into from
Jan 13, 2022

Conversation

devinrsmith
Copy link
Member

From a suggestion from @niloc132 in #1787, I decided to plumb AutoService for better maintainability and visibility into ServiceLoader services. This reduces the developer burden for creating the services files manually by using https://github.com/google/auto/tree/master/service - this is a compile/annotation-time-only dependency, no additional runtime dependencies. In addition, it likely fares better during refactoring, and provides immediate context to anybody looking at the code that this will be auto-discovered during runtime.

As part of the PR, I noticed that open-api/lang-parser/src/main/resources/META-INF/services/io.deephaven.lang.parse.api.CompletionParseService is not a real service (maybe it used to be?).

Copy link
Member

@rcaudy rcaudy left a comment

Choose a reason for hiding this comment

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

What about deephaven-core/py/jpy/src/test/resources/META-INF/services/javax.script.ScriptEngineFactory ?

fishconfig-local/build.gradle Outdated Show resolved Hide resolved
@devinrsmith
Copy link
Member Author

Preferring to not push down into jpy just to minimize churn there.

@devinrsmith devinrsmith merged commit b740bb3 into deephaven:main Jan 13, 2022
@devinrsmith devinrsmith deleted the auto-service branch January 13, 2022 15:58
@github-actions github-actions bot locked and limited conversation to collaborators Jan 13, 2022
@JamesXNelson
Copy link
Member

Indeed, it started as a service, then colin refactored it to not need to be a service anymore. Feel free to rename / delete as you see fit.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants