Skip to content

Conversation

@adriendupuis
Copy link
Contributor

@adriendupuis adriendupuis commented Feb 7, 2023

Question Answer
JIRA issue N/A
Type bug
Target Ibexa version v4.4.x
BC breaks no

Changes suggested by https://www.php.net/manual/en/migration82.deprecated.php#migration82.deprecated.core.dollar-brace-interpolation

Avoid the following warnings when running on PHP 8.2:

  • Deprecated: Using ${var} in strings is deprecated, use {$var} instead in /var/www/vendor/ibexa/core/src/lib/Persistence/Cache/ContentHandler.php on line 134
  • Deprecated: Using ${var} in strings is deprecated, use {$var} instead in /var/www/vendor/ibexa/core/src/lib/Persistence/Cache/ContentHandler.php on line 241

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly (main for features, the oldest supported for bugs).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review (ping @ibexa/engineering).

For PHP 8.2 https://www.php.net/manual/en/migration82.deprecated.php#migration82.deprecated.core.dollar-brace-interpolation

Deprecated: Using ${var} in strings is deprecated, use {$var} instead in /var/www/vendor/ibexa/core/src/lib/Persistence/Cache/ContentHandler.php on line 134

Deprecated: Using ${var} in strings is deprecated, use {$var} instead in /var/www/vendor/ibexa/core/src/lib/Persistence/Cache/ContentHandler.php on line 241
@Steveb-p Steveb-p requested a review from a team February 7, 2023 10:56
@konradoboza konradoboza requested a review from a team February 7, 2023 10:58
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

Congrats on your first contribution to the Core @adriendupuis 🎉
The change is good as long as our Continuous Integration in the PR passes.
However there is a bit of standardization that applies to Product PRs, especially core

First of all, do you want to see this change in 4.3, 4.4, or the upcoming 4.5 (end of March)? This is important because we need to know when and to which branch to merge it.

Secondly, PR title should be formed in past simple, stating what has been changed, for instance:

Fixed deprecated var interpolation style in Caching ContentHandler

because once merged, that message describes history.

Side: Usually PR title should be prefixed with IBX JIRA issue number followed by colon, but for such small change in this case it's not necessary.

Copy link
Contributor

@mikadamczyk mikadamczyk left a comment

Choose a reason for hiding this comment

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

A commit message could be more descriptive

@adriendupuis adriendupuis changed the title ContentHandler.php: ${versionNo} → {$versionNo} Fixed deprecated var interpolation style in Caching ContentHandler Mar 30, 2023
@adriendupuis
Copy link
Contributor Author

Duplicate #205

@adriendupuis adriendupuis deleted the adrien-patch-php82 branch March 30, 2023 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants