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

refactor: default providers #1974

Merged
merged 2 commits into from
Sep 19, 2022
Merged

refactor: default providers #1974

merged 2 commits into from
Sep 19, 2022

Conversation

ndr-brt
Copy link
Member

@ndr-brt ndr-brt commented Sep 16, 2022

What this PR changes/adds

Cleanup default providers following the documentation guidelines

Why it does that

Project structure review

Further notes

  • Put default services in dedicated default extensions
  • Removed DidStore as it wasn't used

Linked Issue(s)

Closes #1812

Checklist

  • added appropriate tests?
  • performed checkstyle check locally?
  • added/updated copyright headers?
  • documented public classes/methods?
  • added/updated relevant documentation?
  • assigned appropriate label? (exclude from changelog with label no-changelog)
  • formatted title correctly? (take a look at the CONTRIBUTING and styleguide for details)

@ndr-brt ndr-brt added the refactoring Cleaning up code and dependencies label Sep 16, 2022
@ndr-brt ndr-brt temporarily deployed to Azure-dev September 16, 2022 09:34 Inactive
@ndr-brt ndr-brt temporarily deployed to Azure-dev September 16, 2022 09:46 Inactive
@codecov-commenter
Copy link

Codecov Report

Merging #1974 (c839aea) into main (e25e01a) will decrease coverage by 0.02%.
The diff coverage is 54.28%.

@@            Coverage Diff             @@
##             main    #1974      +/-   ##
==========================================
- Coverage   62.93%   62.91%   -0.03%     
==========================================
  Files         784      786       +2     
  Lines       16635    16599      -36     
  Branches     1081     1078       -3     
==========================================
- Hits        10470    10443      -27     
+ Misses       5714     5706       -8     
+ Partials      451      450       -1     
Impacted Files Coverage Δ
...ore/controlplane/DefaultControlPlaneExtension.java 0.00% <0.00%> (ø)
.../catalog/cache/FederatedCatalogCacheExtension.java 100.00% <ø> (ø)
...ataspaceconnector/api/DefaultApiCoreExtension.java 0.00% <0.00%> (ø)
...connector/azure/cosmos/DefaultCosmosExtension.java 0.00% <0.00%> (ø)
...aceconnector/iam/did/IdentityDidCoreExtension.java 0.00% <ø> (ø)
...dataplane/sync/DataPlaneTransferSyncExtension.java 0.00% <ø> (ø)
...ne/sync/DefaultDataPlaneTransferSyncExtension.java 0.00% <0.00%> (ø)
.../transfer/provision/http/HttpWebhookExtension.java 82.75% <ø> (-1.12%) ⬇️
.../dataspaceconnector/core/DefaultCoreExtension.java 55.55% <55.55%> (ø)
...atalog/cache/DefaultFederatedCatalogExtension.java 75.00% <75.00%> (ø)
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ndr-brt ndr-brt temporarily deployed to Azure-dev September 16, 2022 10:01 Inactive
*/
public class DefaultServicesExtension implements ServiceExtension {
public class DefaultCoreExtension implements ServiceExtension {
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: how about naming these types of extensions Default...ervicesExtension, since they provide default core services...? Naming may get overly long at times though.

Copy link
Member Author

Choose a reason for hiding this comment

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

@paullatzelsperger Actually that's a good advice, I renamed all these extensions in the *DefaultServicesExtension format and same for the name() method return value: * Default Services. Makes everything clearer.

@ndr-brt ndr-brt temporarily deployed to Azure-dev September 19, 2022 07:24 Inactive
@ndr-brt ndr-brt temporarily deployed to Azure-dev September 19, 2022 07:29 Inactive
@bscholtes1A
Copy link
Contributor

Awesome!

@ndr-brt ndr-brt merged commit aa2486d into eclipse-edc:main Sep 19, 2022
@ndr-brt ndr-brt deleted the feature/1812-default-providers branch September 19, 2022 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Cleaning up code and dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Project strucure: move @Provider(isDefault = true) service providers to the respective core extension
4 participants