Skip to content

Conversation

@jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Dec 10, 2024

The test as added in PR #170 did not actually hit the PHP 8.4 deprecation notice, which - while it is a useful test for other reasons - makes it an ineffective test for safeguarding the fix from #170, which was about preventing the PHP 8.4 deprecation notice.

This commit updates the test to ensure the fix for the PHP 8.4 deprecation notice is now properly safeguarded.

This can be verified by undoing the change to the src/CodeManipulation.php file from #170 and running this test on PHP 8.4. The test will fail and show the deprecation notice. When the change from #170 is re-applied, the test will pass without the deprecation notice.

Includes adding the ===DONE=== markers for consistency with the other tests.

The test as added in PR 170 did not actually hit the PHP 8.4 deprecation notice, which - while it is a useful test for other reasons - makes it an ineffective test for safeguarding the fix from 170, which was about preventing the PHP 8.4 deprecation notice.

This commit updates the test to ensure the fix for the PHP 8.4 deprecation notice is now properly safeguarded.

This can be verified by undoing the change to the `src/CodeManipulation.php` file from 170 and running this test on PHP 8.4. The test will fail and show the deprecation notice.
When the change from 170 is re-applied, the test will pass without the deprecation notice.

Includes adding the `===DONE===` markers for consistency with the other tests.
@jrfnl jrfnl added this to the 2.2 Next milestone Dec 10, 2024
@jrfnl jrfnl requested a review from antecedent December 10, 2024 16:30
@jrfnl jrfnl mentioned this pull request Dec 10, 2024
@jrfnl
Copy link
Collaborator Author

jrfnl commented Dec 10, 2024

@antecedent Might be good to tag a new patch version once this PR has been merged as I can imagine other projects working on their PHP 8.4 readiness running into the deprecation notice.

@antecedent
Copy link
Owner

Oops. Great that you caught this, @jrfnl.

And yes, I agree that a new patch version is a good idea. I will tag one.

@antecedent antecedent merged commit 1bf183a into master Dec 11, 2024
20 checks passed
@jrfnl jrfnl deleted the feature/getcachedpath-fix-test branch December 11, 2024 10:34
@jrfnl
Copy link
Collaborator Author

jrfnl commented Dec 11, 2024

Thanks @antecedent! (also for the release)

@jrfnl jrfnl modified the milestones: 2.2 Next, 2.2.1 Sep 17, 2025
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