Repository navigation
Use require/require_once for all file inclusions - #136
Merged
Merged
Conversation
Switches every include/include_once to require/require_once for fail-fast consistency. Schema already lives in includes/setup/database.php via api_plugin_db_table_create()/gpsmap_upgrade_database(), and the library code is already under includes/, so no schema or file-move changes were needed.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Three remaining test inclusions contradict the stated repository-wide require-only convention.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Replaces production PHP inclusions with fail-fast require variants and documents the convention.
Changes:
- Converts plugin entry-point, setup, and polling inclusions.
- Updates repository guidance and changelog.
- Leaves three test-file
includecalls inconsistent with the new absolute rule.
| File | Description |
|---|---|
setup.php |
Requires database setup dependencies. |
includes/setup/database.php |
Requires database and plugin setup files. |
includes/polling/processregion.php |
Requires the host class. |
includes/polling/kmlcreation.php |
Requires KML icon helpers. |
includes/polling/functions.php |
Requires polling dependencies. |
includes/polling.php |
Requires poller implementation files. |
gpstemplates.php |
Requires authentication and security guards. |
gpsmap.php |
Requires authentication, setup, and icon files. |
CHANGELOG.md |
Records the inclusion-policy change. |
.github/copilot-instructions.md |
Establishes the require-only convention. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
added 5 commits
September 29, 2026 21:51
The patch-coverage gate requires every changed production PHP file to appear in Clover (or be allowlisted) and 100% of changed statement lines in measured files to be covered. Adds the web/poller entry points that cannot be loaded in the isolated unit process to tests/bin/patch-coverage.php's allowlist, and reverts the one include->require change in includes/polling/functions.php (callRegion() is a live-poller-only helper with no unit coverage).
…er include->require The patch-coverage gate requires every changed production PHP file to be in Clover or allowlisted, and 100% of changed statement lines in measured files to be covered. Keeps the require/require_once conversion on the web entry points (gpsmap.php, gpstemplates.php), the hook-registration bootstrap (setup.php) and the install/upgrade bootstrap (includes/setup/database.php) - all unmeasurable in the isolated unit process and therefore allowlisted - and reverts the include->require swaps in the live-poller-only internals (includes/polling.php, includes/polling/kmlcreation.php, includes/polling/processregion.php) whose changed lines only execute inside a real poller run.
…cludes-database # Conflicts: # CHANGELOG.md
…y with the require sweep
TheWitness
requested review from
bmfmancini,
browniebraun,
cigamit and
xmacan
September 30, 2026 02:54
Member
Author
|
This branch now also sets the plugin's |
browniebraun
previously approved these changes
Sep 30, 2026
added 5 commits
September 30, 2026 19:55
phpunit.xml ships in the repo for CI but is a dev-only test artifact, so it is removed from manifest.json 'expected', pruned from installs on upgrade (like tests/), and excluded from the manifest drift check. The retired markdown-lint configs (.mdlrc, .md_style.rb) are deleted, and .md* is now ignored like .git*: protected from pruning and excluded from drift if it reappears.
The upgrade-time prune now rejects any tombstone containing '.'/'..' segments (which could escape the plugin directory or resolve to its root) and treats ancestors of whitelist entries as protected, so a tombstone on a parent directory can no longer delete a whitelisted file beneath it.
The trailing '# ...' comments in the Project Structure block drifted further right down the tree; align them all to a single column.
- Use the full license header (from the plugin's own setup.php) in tests/Unit/PruneFilesTest.php instead of the abbreviated copyright banner. - Document that the manifest drift check and the upgrade-time prune also handle phpunit.xml and .md* files, matching the implemented behavior.
The repository naming contract reserves the plugin_<name>_ prefix for plugin lifecycle / hook-registration functions; all other functions use the plain <name>_ prefix. Rename the internal upgrade helpers accordingly: plugin_<name>_prune_files() -> <name>_prune_files() plugin_<name>_rmtree() -> <name>_rmtree() The call site, unit tests, and the copilot-instructions.md references are updated to match. No behavioral change.
cigamit
approved these changes
Oct 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part of a fleet-wide consistency pass. This plugin already follows the target layout — its library code is under
includes/and its schema lives inincludes/setup/database.phpusingapi_plugin_db_table_create()with agpsmap_upgrade_database()upgrade path — so this PR combines theinclude→requiresweep with the fleetmanifest.json+ upgrade-time file-pruning mechanism.require over include
Every
include/include_onceacrossgpsmap.php,gpstemplates.php,setup.php,includes/setup/database.php, and theincludes/polling/*files is nowrequire/require_oncefor fail-fast consistency. Paths are unchanged (no relocations).gpsmap_security.phpintentionally stays in the plugin root — it's referenced as a root-level guard by ~8 files (including via__DIR__ . '/../...'fromincludes/subfiles) and is symlinked as a root file by the test harness.File manifest + upgrade pruning
manifest.jsonwithtombstones(empty — this plugin has no relocations),expected(the top-level files/directories shipping today, directories with a trailing/), andwhitelist(empty —images/ships stock marker icons, there is no user-writable runtime tree).gpsmap_prune_files()(insetup.php, called from the version-change block ofgpsmap_check_upgrade()right aftergpsmap_upgrade_database()) removes the dev-onlytests/tree on upgrade, refuses any path that resolves outside the plugin directory (a tamperedmanifest.json), warns in the Cacti log on any file/directory it cannot remove, leaveswhitelist/.git*alone, and logs — without removing — any unaccounted top-level entry.tests/bin/validate-manifest.php(wired intoplugin-ci-workflow.yml) fails on drift betweenexpectedand the real top-level tree.tests/Unit/PruneFilesTest.php. The isolated-processtests/Integration/UpgradeTest.phpprobe already runs under a tempbase_path(gpsmap_test_use_tmp_root()), so itscheck_upgrade()calls find no manifest and the prune is a no-op there.Docs
.github/copilot-instructions.md: doc example updated torequire_once, plus a require-over-include convention bullet and a manifest/prune note. CHANGELOG entry added.Validation
php -lclean on all changed PHP files; full Pest suite green (147 passed, the 9 risky cases are pre-existing assertion-free Polling/Integration probes) and patch-coverage passes (thesetup.phpchanges are in the established unmeasured allowlist); manifest drift-check passes and the translation template is up to date.Revision: hardening & fleet cleanup
Since the initial description, this PR also:
phpunit.xmlon upgrade (alongside the dev-onlytests/tree) and leaves.md*lint configs in place — the drift check now ignorestests/,phpunit.xml,.git*,.md*, and whitelisted paths../..traversal segment or resolves outside the plugin directory (including via a symlinked directory), and protects a directory when a whitelisted entry lives beneath it — each with added unit coverage.gpsmap_prune_files()/gpsmap_rmtree()(theplugin_gpsmap_prefix is reserved for lifecycle/hook-registration functions).tests/Unit/PruneFilesTest.phpthe full standard GPL v2 header and aligns the Project Structure block in.github/copilot-instructions.md.