Thank you all for your review, feedback, and help to improve the module!
Hi @vishal.kadam
Thank you very much for your review.
1. VERSION
I've replaced the VERSION constant with our version number.
2. Constructor documentation comments
I removed the constructor documentation comments since they're not mandatory.
3. Hook documentation comments
I've adjusted the comments for the hooks.
One of the hooks you listed was not a hook but a normal function. I've moved this functionality into the object where we used it since there is no reason to have it in a separate function. With that, we were additionally able to eliminate one \Drupal call.
Kind regards,
zepich
Hi @solideogloria
Thank you very much for your feedback!
accessCheck
Thank you very much. I've missed some occurrences before and have now adjusted all the queries.
eslint result
Thank you very much. Unfortunately, this is an issue of the Eslint ruleset since Eslint thinks these three lines are arrays/objects, and we have to add a comma at the end. But in reality, these three issues are multi-line function calls. Adding a comma at the end will break the function.
I've eliminated one of the lines by creating a constant, but I don't see any option to solve the others. If you have an idea, please let me know.
MosparoService interface
As suggested, I've added an interface and replaced all the type hints with the interface.
Kind regards,
zepich
Hi @rushikesh-raval
Thank you very much for your feedback.
Which version have you tested? All the things you found are fixed in the latest commit (https://git.drupalcode.org/project/mosparo_integration/-/tree/483264de07...) (but not in the Beta version from last year).
You can also see the phpcs result in the GitLab CI build from last Sunday: https://git.drupalcode.org/project/mosparo_integration/-/jobs/744569
Kind reagrds,
zepich
zepich → created an issue.
I fixed one \Drupal usage. The other uses are not fixable in my view.
Hi @urvashi_vora
Thank you very much for your issue and the patch.
I've reviewed it and adjusted it. In my eyes, we cannot replace the \Drupal call in the MosparoWidget because it uses the constructor method from WidgetBase which is not able to accept Dependency Injection (https://git.drupalcode.org/project/mosparo_integration/-/blob/1.0.x/modu...).
I'm open to suggestions if you know how we could solve that. We have the same problem in the other module (https://git.drupalcode.org/project/mosparo_integration/-/blob/1.0.x/modu...) which the CodeSniffer didn't found.
Thank you very much, and have a nice day!
Kind regards,
zepich