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

Use EntityTypeManagerInterface declaration instead of EntityTypeManager (Nitpick) #4296

Closed
stefan-korn opened this issue Sep 25, 2024 · 2 comments · Fixed by #4297
Closed
Labels

Comments

@stefan-korn
Copy link
Contributor

Current Behavior

It seems more approriate to use EntityTypeManagerInterface as declaration type for the entityTypeManager property in Drupal\metastore\Nodewrapper\Data.

Otherwise for example the call to new Data in _metastore_search_is_dataset is complaining about the declaration type.

This surely is a nitpick, but since IDE is constantly reminding me on this ...

Expected Behavior

no complaining about declaration type anymore

Steps To Reproduce

let your IDE check the call to new Data in _metastore_search_is_dataset

Relevant log output (optional)

No response

Anything else?

No response

@paul-m
Copy link
Contributor

paul-m commented Sep 26, 2024

Thanks.

I think your nitpick is 100% correct, and we should change that. We should also be nitpicky and fix NodeDataFactory in a similar way. (Inject interfaces instead of concrete classes.) Go ahead and add that to the PR if you'd like.

stefan-korn added a commit to stefan-korn/dkan that referenced this issue Sep 27, 2024
@stefan-korn
Copy link
Contributor Author

Okay, added NodeDataFactory and some other places too.

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

Successfully merging a pull request may close this issue.

2 participants