Closed (fixed)
Project:
Sortableviews
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
14 Apr 2026 at 14:12 UTC
Updated:
17 Jul 2026 at 12:20 UTC
Jump to comment: Most recent
Comments
Comment #4
sapnil_biswas commented@anybody marking for review, please have a look at it when you have time
Comment #5
anybodyThanks @sapnil_biswas. I left some comments. Some of these changes are unclear to me. Was it done using AI?
Comment #6
anybodyComment #7
sapnil_biswas commented@anybody Thanks for the feedback. The PR wasn't generated through AI, I tried review it through AI once that might have caused the errors i will get them rectified
Comment #8
sapnil_biswas commentedI have force-pushed the version before the AI review please let me know if there is anything over the board or requires refinement
Comment #11
dimilias commented@anybody, normally I should not take novice ticket, I am just showcasing to some colleagues how the ddev and ddev modul contrib add-on works (mentoring for DDD2026) so I took it up to fix the minor issues.
Comment #12
anybodyThanks @dimilias what about the existing MR!15?
Comment #13
dimilias commented@anybody I don't really mind. I didn't want to write on top of that. The AI review indeed shows that the code can be further improved. I provided only the minimum required changes for tests to go green and I didn't want to step on the previous MR only because there are changes there I find uneeded (and possibly LLM assisted :/)
To be fair though, as I said, this is part of mentoring work and I wouldn't mind if I didn't even receive credit for this. It can go to sapnil_biswas, it is up to you. Or as part of demonstration, you can even disregard my MR with all due respect.
:) Leaving it to you.
Comment #14
dimilias commentedComment #15
dimilias commentedbtw, @anybody, please, feel free to refactor my MR on top of MR!15. I can quickly do that.
Comment #16
anybodyThanks @dimilias
Yes it would be great to have a best practice final one, which I can review and merge simply. Of course, you'll all be credited!
Thank you for your help!
Comment #18
dimilias commented@anybody I have updated the current branch.
Notes:
* I only kept the bare minimum necessary to fix the tests.
* I kept, however, a small typo which is in the phpdoc and don't think it makes a difference.
* Even though the previous commit had attribute declaration for the dataprovider, I kept the legacy @dataProvider because the module does not have a composer.json and the info file shows support for ^8 so I wanted to be on the safe side.
* About
, I forgot to add a 2 words that makes it much different. It is "feel free to ASK ME to refactor.." :P
Comment #19
anybodyI think this should be reviewed and merged with priority.
Comment #21
dieterholvoet commentedThanks!