Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the installation process by updating minimum PHP and XOOPS version requirements and standardizing database operations to use the exec() method for write operations (INSERT, UPDATE, DELETE, DDL) instead of the query() method.
Changes:
- Updated minimum PHP version requirement from 5.6.0 to 7.4 and minimum XOOPS version from 2.5.11 to 2.5.12
- Replaced
query()withexec()for all mutating database operations across installation, uninstallation, and core model files - Removed conditional logic that previously chose between
query()andexec()based on force flags, now consistently usingexec()for write operations
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| htdocs/xoops_lib/modules/protector/xoops_version.php | Updated minimum PHP version to 7.4 and minimum XOOPS version to 2.5.12 |
| htdocs/xoops_lib/modules/protector/onuninstall.php | Changed DROP TABLE query from query() to exec() |
| htdocs/xoops_lib/modules/protector/oninstall.php | Changed CREATE TABLE query from query() to exec() |
| htdocs/modules/system/admin/modulesadmin/modulesadmin.php | Changed table creation query from query() to exec() |
| htdocs/modules/profile/include/install.php | Changed INSERT queries from query() to exec() |
| htdocs/class/model/write.php | Standardized all write operations (INSERT, UPDATE, DELETE) to use exec() method with commented-out old conditional logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // $queryFunc = empty($force) ? 'query' : 'exec'; | ||
| $queryFunc = 'exec'; |
There was a problem hiding this comment.
These commented-out lines should be removed rather than kept as comments. The old logic that conditionally chose between 'query' and 'exec' based on the $force parameter has been replaced with always using 'exec', which is the correct approach for DELETE operations. Leaving commented code reduces maintainability.
| // $queryFunc = empty($force) ? 'query' : 'exec'; | ||
| $queryFunc = 'exec'; |
There was a problem hiding this comment.
These commented-out lines should be removed rather than kept as comments. The old logic that conditionally chose between 'query' and 'exec' based on the $force parameter has been replaced with always using 'exec', which is the correct approach for UPDATE operations. Leaving commented code reduces maintainability.
| $modversion['module_website_name'] = 'XOOPS'; | ||
| $modversion['min_php'] = '5.6.0'; | ||
| $modversion['min_xoops'] = '2.5.11'; | ||
| $modversion['min_php'] = '7.4'; |
There was a problem hiding this comment.
The version string '7.4' is inconsistent with the previous format '5.6.0' and other modules in the codebase that use three-component version strings (e.g., '5.6.0' in pm/xoops_version.php and profile/xoops_version.php). While functionally equivalent in version_compare(), it's better to maintain consistency with the established format. Consider using '7.4.0' instead.
| $modversion['min_php'] = '7.4'; | |
| $modversion['min_php'] = '7.4.0'; |
| // $queryFunc = empty($force) ? 'query' : 'exec'; | ||
| $queryFunc = 'exec'; |
There was a problem hiding this comment.
These commented-out lines should be removed rather than kept as comments. The old logic that conditionally chose between 'query' and 'exec' based on the $force parameter has been replaced with always using 'exec', which is the correct approach for DELETE operations. Leaving commented code reduces maintainability.
1 participant
No description provided.