- 🇫🇷France O'Briat Nantes
I add it to the global issue #3377377 about drupal-check
- Status changed to Needs work
9 months ago 3:37pm 3 October 2023 - Status changed to Needs review
9 months ago 5:31pm 11 October 2023 - last update
9 months ago PHPLint Failed - last update
9 months ago 6 pass - Status changed to Needs work
9 months ago 6:55pm 11 October 2023 - 🇺🇸United States TR Cascadia
None of the "TODO" comments should be in there. If they are valid things that need to be addressed in the future, then they should use the proper format @todo so we can find these comments in the future when we look for them. AND if a @todo is added, then there must ALSO be a new issue opened so that this to-do item can be tracked in the issue queue. It makes no sense, and doesn't help this module move forward, just to add code comments about things that look wrong or might need to be explained better. If it needs to be explained better, then add a code comment to explain it better - that would be the proper fix.
This patch also splits one line and declares a new variable so a third @var comment can be added. That's neither good nor bad, but it's not required by drupal-check.
The rest of the patch is just the "TODO" comments which aren't helping anything. The comment about ->rating is good - that should be a BUG report handled in a new issue. The comment about the IP address is something that should be discussed as a feature request. Neither of those comments should be in this patch.
This patch has only two useful lines - the two added @var comments. And in general, if @var comments are going to be added to satisfy drupal-check warnings then they should be added everywhere at once, not just in this one file. We don't need individual patches for each individual file to add @var comments. We don't need to review individual patches for individual files that contain @var comments PLUS a bunch of other unrelated changes. One patch, with all the @var comments for the entire module AND NOTHING ELSE would be simpler and quicker to review. It would also be useful because it adds documentation to the code and makes it easier to understand and maintain and modify in the future.
- 🇮🇳India sanjayk
@TR As you mentioned I have created a new issue https://www.drupal.org/project/fivestar/issues/3393492 🐛 Add @todo comment in project Needs work .