Account created on 3 February 2009, over 15 years ago
#

Recent comments

πŸ‡¨πŸ‡­Switzerland zepich

Thank you all for your review, feedback, and help to improve the module!

πŸ‡¨πŸ‡­Switzerland zepich

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

πŸ‡¨πŸ‡­Switzerland 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

πŸ‡¨πŸ‡­Switzerland 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

πŸ‡¨πŸ‡­Switzerland zepich

I fixed one \Drupal usage. The other uses are not fixable in my view.

πŸ‡¨πŸ‡­Switzerland zepich

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

Production build 0.69.0 2024