Skip to content

Chore self to static refactor#78

Merged
Meldiron merged 3 commits into
mainfrom
chore-self-to-stati
Mar 23, 2026
Merged

Chore self to static refactor#78
Meldiron merged 3 commits into
mainfrom
chore-self-to-stati

Conversation

@Meldiron

Copy link
Copy Markdown
Contributor

Will be useful once we add more adapters

@greptile-apps

greptile-apps Bot commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR refactors all self:: static property accesses to static:: in GiteaTest.php, enabling Late Static Binding (LSB) as a foundation for supporting future test adapters. The change is low-risk and functionally equivalent for the current single test class, but requires one follow-up to be fully effective.

  • All self::$accessToken and self::$owner references in GiteaTest are consistently updated to static::$accessToken / static::$owner across setUp(), setupGitea(), and every test method.
  • The two static properties ($accessToken and $owner) are still declared as private static, which means LSB has no practical effect — static:: on a private property always resolves to the declaring class, regardless of the runtime class. For the stated goal of supporting subclass adapters, these properties should also be changed from private to protected.

Confidence Score: 4/5

  • Safe to merge — the change is functionally a no-op for the existing test class; however, the intended benefit (supporting subclass adapters) will only materialise once the property visibility is updated to protected.
  • The refactor is mechanical and consistent. It does not break any existing tests because static:: on private properties resolves identically to self:: within the same class. One minor follow-up is needed — changing the visibility from private to protected — to make the LSB refactor actually useful for future subclasses.
  • tests/VCS/Adapter/GiteaTest.php — property visibility should be protected static to complement the LSB refactor.

Important Files Changed

Filename Overview
tests/VCS/Adapter/GiteaTest.php Refactors all self:: accesses of static properties to static:: (Late Static Binding), but the two static properties remain private, which means LSB provides no real benefit yet; changing their visibility to protected is needed to fulfil the PR's intent of supporting future adapters/subclasses.

Comments Outside Diff (1)

  1. tests/VCS/Adapter/GiteaTest.php, line 14-15 (link)

    P2 static:: has no effect on private properties

    The PR's stated goal is to support future subclasses ("Will be useful once we add more adapters"), which is exactly why static:: (Late Static Binding) is being introduced. However, both $accessToken and $owner are declared as private static, which means:

    1. They are not inherited by subclasses — a subclass cannot see or share these properties.
    2. static::$accessToken inside a method of GiteaTest will always resolve to GiteaTest::$accessToken, making the LSB change a no-op for the current class.
    3. If a future subclass is introduced, static::$accessToken in the inherited setUp() would attempt to resolve against the child class, where the property doesn't exist — potentially causing unexpected behavior or errors.

    To fulfil the stated intent, these properties should be changed to protected static:

    This allows subclasses to inherit and override the values independently, which is exactly the use-case LSB is designed for.

Reviews (1): Last reviewed commit: "Fix syntax error" | Re-trigger Greptile

@Meldiron Meldiron merged commit 35ded61 into main Mar 23, 2026
3 checks passed
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.

1 participant