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(iam): remove unused dummy-credentials-verifier #2095

Merged

Conversation

juliapampus
Copy link
Contributor

@juliapampus juliapampus commented Oct 13, 2022

What this PR changes/adds

Remove :dummy-credentials-verifier

Why it does that

Not used anywhere in EDC repositories

Further notes

--

Linked Issue(s)

Closes #2094

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)

@juliapampus juliapampus added the refactoring Cleaning up code and dependencies label Oct 13, 2022
@juliapampus juliapampus temporarily deployed to Azure-dev October 13, 2022 14:21 Inactive
@juliapampus juliapampus temporarily deployed to Azure-dev October 13, 2022 14:29 Inactive
Copy link
Member

@ndr-brt ndr-brt left a comment

Choose a reason for hiding this comment

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

Ok with this, only question, shouldn't we provide a default implementation for the CredentialsVerifier? Otherwise it won't be possible to use the DecentralizedIdentityServiceExtension without providing a custon extension

@juliapampus
Copy link
Contributor Author

@ndr-brt So this module is used as default impl right now? Then I will not remove it and rename it instead.

@ndr-brt
Copy link
Member

ndr-brt commented Oct 14, 2022

No, is not provided by default, but I think we should "always" have a default implementation for every service (except maybe some more "crucial", as discussed in #2042 ), maybe this is something do be done in the future.

TL;DR: let's get this merged :D

@juliapampus juliapampus merged commit 6623720 into eclipse-edc:main Oct 14, 2022
@juliapampus juliapampus deleted the 2094_remove_dummy_verifier branch October 14, 2022 09:25
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.

Remove module dummy-credentials-verifier
3 participants