Skip to content

Conversation

@acurrieclark
Copy link
Member

No description provided.

Copy link

Copilot AI left a comment

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 merges upstream changes that modernize the Auryn dependency injection library for PHP 7.1+ compatibility. The changes include namespace consolidation, code style improvements, and test infrastructure updates.

  • Updates minimum PHP version requirement from 5.3 to 7.1
  • Consolidates namespaces from Auryn\Test to Auryn for test fixtures
  • Modernizes CI/CD infrastructure from Travis CI to GitHub Actions

Reviewed Changes

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

Show a summary per file
File Description
test/fixtures_5_6.php Updates namespace from Auryn\Test to Auryn
test/fixtures.php Updates namespace, adds missing property declarations, fixes spelling errors, and modernizes PHP syntax
test/InjectorTest.php Updates namespace, modernizes test methods with void return types and PHPUnit assertions
test/Benchmark/Noop.php Updates namespace and fixes spelling in comments
test/Benchmark/ExecuteBenchmark.php Updates namespace from Auryn\Test\Benchmark to Auryn\Benchmark
phpunit.xml Removes deprecated syntaxCheck attribute
lib/StandardReflector.php Improves type hint detection for PHP 8+ and adds builtin type filtering
lib/Reflector.php Updates return type annotation to include null
lib/ReflectionCacheApc.php Adds nullable type hints and fixes constructor call
lib/Injector.php Adds public visibility to constants, improves type hints, and fixes edge cases
lib/InjectionException.php Adds nullable type hints to exception parameters
lib/CachingReflector.php Adds public visibility to constants and nullable type hints
composer.json Updates PHP version requirement and test dependencies
composer-require-check.json Adds new configuration file for dependency checking
.travis.yml Removes Travis CI configuration
.php_cs Removes old PHP CS Fixer configuration
.php-cs-fixer.php Adds new PHP CS Fixer configuration
.github/workflows/ci.yml Adds GitHub Actions CI workflow

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

private function make()
{
$obj = new \StdClass;
$obj = new \stdClass();
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

The class name should be StdClass (uppercase 'S') to match PHP's standard naming convention for built-in classes.

Suggested change
$obj = new \stdClass();
$obj = new \StdClass();

Copilot uses AI. Check for mistakes.
{
public $dependency;
public function __construct(\StdClass $instance = null)
public function __construct(?\StdClass $instance = null)
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

The class name should be StdClass (uppercase 'S') to match PHP's standard naming convention for built-in classes.

Suggested change
public function __construct(?\StdClass $instance = null)
public function __construct(?\stdClass $instance = null)

Copilot uses AI. Check for mistakes.
private function normalizeName($className)
{
return ltrim(strtolower($className), '\\');
return ltrim(strtolower($className), '\\?');
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

The ltrim function should only strip leading backslashes, not question marks. The '?' character should not be included in the character mask as it's not a valid namespace separator.

Suggested change
return ltrim(strtolower($className), '\\?');
return ltrim(strtolower($className), '\\');

Copilot uses AI. Check for mistakes.
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.

4 participants