Skip to content

refactor: extract the duplicated timespan lookup in cycle.php - #30

Draft
somethingwithproof wants to merge 1 commit into
Cacti:developfrom
somethingwithproof:refactor/modernization
Draft

refactor: extract the duplicated timespan lookup in cycle.php#30
somethingwithproof wants to merge 1 commit into
Cacti:developfrom
somethingwithproof:refactor/modernization

Conversation

@somethingwithproof

@somethingwithproof somethingwithproof commented Apr 9, 2026

Copy link
Copy Markdown
Member

cycle_graphs() and cycle() built the graph timespan with the same three lines. This moves them into cycle_get_timespan() and calls it from both.

Also drops a stray space before a comma at the second call site.

Rebuilt on current develop. The earlier version of this branch predated #27, which had already converted the array syntax, so most of it was redundant and it no longer rebased cleanly.

No behaviour change.

Copilot AI review requested due to automatic review settings April 9, 2026 21:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modernizes the Cycle plugin’s PHP files by enabling strict typing and updating some legacy syntax to PHP 7.4-era constructs.

Changes:

  • Added declare(strict_types=1); across plugin PHP entrypoints.
  • Converted some array(...) usages to short array syntax ([...]) and updated prepared-statement parameter arrays accordingly.
  • Introduced two .omc/sessions/*.json files (appear to be local tooling artifacts).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
setup.php Adds strict typing and updates a couple of arrays to short syntax.
functions.php Adds strict typing and partially modernizes arrays/params in several functions.
cycle.php Adds strict typing and updates a few array() initializations / prepared params.
index.php Adds strict typing to the plugin redirect entrypoint.
images/index.php Adds strict typing to the redirect stub.
locales/index.php Adds strict typing to the redirect stub.
locales/LC_MESSAGES/index.php Adds strict typing to the redirect stub.
.omc/sessions/859688d5-ece7-4d1e-ab13-c82d820e7440.json New session JSON file (likely unintended).
.omc/sessions/42a5a3ed-3430-434f-addf-dc7423bda6d8.json New session JSON file (likely unintended).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread functions.php Outdated
Comment on lines 118 to 123
$filters = array(
'id' => array(
'id' => [
'filter' => FILTER_VALIDATE_INT,
'default' => '-1'
),
],
'tree_id' => array(

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In validate_request_vars(), the $filters structure now mixes short array syntax ([]) with legacy array() in the same literal, which conflicts with the PR goal of using short array syntax across the plugin and makes the block harder to scan. Convert the remaining nested array(...) entries within $filters to [...] for consistency.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +8
{
"session_id": "859688d5-ece7-4d1e-ab13-c82d820e7440",
"ended_at": "2026-04-09T19:59:02.981Z",
"reason": "other",
"agents_spawned": 0,
"agents_completed": 0,
"modes_used": []
} No newline at end of file

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a tooling/session artifact and not plugin source. Please remove .omc/sessions/*.json from the repository and add an ignore rule (e.g., .omc/ or .omc/sessions/) so local session data is not committed.

Suggested change
{
"session_id": "859688d5-ece7-4d1e-ab13-c82d820e7440",
"ended_at": "2026-04-09T19:59:02.981Z",
"reason": "other",
"agents_spawned": 0,
"agents_completed": 0,
"modes_used": []
}

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +8
{
"session_id": "42a5a3ed-3430-434f-addf-dc7423bda6d8",
"ended_at": "2026-04-09T20:11:48.000Z",
"reason": "other",
"agents_spawned": 0,
"agents_completed": 0,
"modes_used": []
} No newline at end of file

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a tooling/session artifact and not plugin source. Please remove .omc/sessions/*.json from the repository and add an ignore rule (e.g., .omc/ or .omc/sessions/) so local session data is not committed.

Suggested change
{
"session_id": "42a5a3ed-3430-434f-addf-dc7423bda6d8",
"ended_at": "2026-04-09T20:11:48.000Z",
"reason": "other",
"agents_spawned": 0,
"agents_completed": 0,
"modes_used": []
}

Copilot uses AI. Check for mistakes.
@somethingwithproof
somethingwithproof marked this pull request as draft April 11, 2026 00:09
@somethingwithproof

Copy link
Copy Markdown
Member Author

Converted to draft to serialize the stack in this repo. Blocked by #27; will un-draft after that merges to avoid cross-PR merge conflicts.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof somethingwithproof changed the title refactor: safe PHP 7.4 modernization refactor: extract the duplicated timespan lookup in cycle.php Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants