Skip to content

Eliminate globals for $RELASES / $OLDRELEASES / $BRANCHES#1931

Open
markrandall wants to merge 1 commit into
php:previewfrom
markrandall:branch-api-extraction
Open

Eliminate globals for $RELASES / $OLDRELEASES / $BRANCHES#1931
markrandall wants to merge 1 commit into
php:previewfrom
markrandall:branch-api-extraction

Conversation

@markrandall

Copy link
Copy Markdown

Attempts to eliminate the dependence on the $RELEASES, $OLDRELEASES, and $BRANCHES global variables that require injecting in at the top level scope to avoid breaking things.

Changes:

  • releases.inc and versions.inc now return their values, as well as retaining the previous global-scope, allowing the new helper to reliably access their contents by requiring them again, and using the return value.
  • Updated bumpRelease to retain returning logic.
  • Added extra normalization to the data returned from the two data sources.
  • Updated everywhere I could find outside of branches.inc that used those old helpers.
  • Wrote unit tests for the new helpers.

Included:

  • Removes requirements to annotate unit tests with code coverage

Comment thread include/branches.inc Outdated
Comment thread src/Releases/Branches.php Outdated
Comment thread src/Releases/Branches.php
static $cache = null;

/* there is no normalisation required here because it's all standard format */
return $cache ??= require __DIR__ . '/../../include/version.inc';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder whether this "caching" is needed, considering the return happens straight from an opcache-cached oparray.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

version.inc is the one for the most recent releases - as such it's optimized for updating hashes, and and uses an IIFE each time it is included to format the data into the normalized form.

Comment thread src/Releases/Branches.php
{
static $cache = null;
return $cache ??= (function () {
$results = [];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same here, and the use of a closure seems odd to just wrap around a single return value, without it being reused.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I find the style personally cleaner than a big if block or double return points.

Comment thread src/Releases/Branches.php
public static function getBranchOverrides(): array
{
static $cache = null;
return $cache ??= require __DIR__ . '/../../include/branch-overrides.inc';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same

@markrandall markrandall force-pushed the branch-api-extraction branch from 5bc2ef7 to 4660988 Compare June 18, 2026 17:16
…thout having to rely on global-scoped variables.
@markrandall markrandall force-pushed the branch-api-extraction branch from 4660988 to a416b4d Compare June 18, 2026 17:23
@markrandall markrandall changed the title Branch api extraction Eliminate globals for $RELASES / $OLDRELEASES / $BRANCHES Jun 18, 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.

3 participants