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

feat: remove MvdScopeTransformer #384

Conversation

thomasrutger
Copy link
Contributor

@thomasrutger thomasrutger commented Nov 6, 2024

What this PR changes/adds

Remove MvdScopeTransformer that contained strange fallback behavior (from "dark times past") when an unrecognized scope credential type is encountered and use the default EdcScopeToCriterionTransformer instead.

Why it does that

This may be an artifact of dark times past, and could probably be removed

Further notes

@paullatzelsperger I did not add any tests, considering none of the mvd contains any tests, but I can add some if desired.

Linked Issue(s)

Closes #383

@paullatzelsperger paullatzelsperger self-requested a review November 6, 2024 13:45
@paullatzelsperger
Copy link
Member

I did not add any tests, considering none of the mvd contains any tests, but I can add some if desired.

this should be covered by the E2E tests we have in MVD

@paullatzelsperger
Copy link
Member

paullatzelsperger commented Nov 7, 2024

the entire MvdScopeTransformer can be deleted

@thomasrutger thomasrutger changed the title feat: remove unrecognized scope fallback feat: remove MvdScopeTransformer Nov 7, 2024
@thomasrutger
Copy link
Contributor Author

the entire MvdScopeTransformer can be deleted

Done. I also updated the readme by moving 8.5 to 10.4 to indicate that the default EdcScopeToCriterionTransformer should probably not be used in a production scenario.

@thomasrutger thomasrutger marked this pull request as ready for review November 7, 2024 14:13
@paullatzelsperger paullatzelsperger merged commit ca48bd9 into eclipse-edc:main Nov 8, 2024
8 checks passed
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.

MvdScopeTransformer question
2 participants