Skip to content

Feature/#610 Configure plugin locations - #611

Draft
kaklakariada wants to merge 6 commits into
mainfrom
feature/#610_configure_plugin_locations
Draft

kaklakariada wants to merge 6 commits into
mainfrom
feature/#610_configure_plugin_locations

Conversation

@kaklakariada

Copy link
Copy Markdown
Contributor

Closes #610

@redcatbear

Copy link
Copy Markdown
Collaborator

61 files. Dude, you are killing me! 🤣

I will do my very best to stay focused on the review.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Curiosity: why are there so many formatter changes in a PR about configuring plugin locations?

* configuration for the service loader
*/
ServiceLoaderFactory(final Path pluginsDirectory, final boolean searchCurrentClasspath)
ServiceLoaderFactory(final ServiceLoaderConfig config)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maintainability: We might want to start marking internal APIs, so that we make sure we can have a clear distinction of what constitutes a breaking change and what does not.

Of the top of my head I would not count this class as part of the public interface. Though I am also 50% sure that someone (maybe even me) used that in their derived project. E.g., the IDE plugin.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

UX: Convention should also be encoded in the project. Not necessarily here. But generally. I don't like configuration. It's a last resort that you need sometimes. Convention however I do like because it make the userguide simpler and improves user experience.

Just to make an example: "$HOME"/.openfasttrace/plugins would be something where we should always look for plugins.

This branch has not been deployed

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

Labels

Projects

Status: 📫 Backlog

Development

Successfully merging this pull request may close these issues.

Allow adding plugins via configuration

2 participants