Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deprecated string interpolation in Analytics.php #7170

Merged
merged 6 commits into from
Sep 16, 2023

Conversation

ridonibishi
Copy link
Contributor

Fixes #7169

Changes proposed in this Pull Request

Title: Fix deprecated string interpolation in Analytics.php

Description:
This PR addresses a deprecation issue in /var/www/html/web/app/plugins/woocommerce-payments/includes/multi-currency/Analytics.php. The problem involves the use of ${var} for string interpolation, which is deprecated. This change replaces ${var} with the correct {$var} format, resolving the following warnings:

  • Deprecated: Using ${var} in strings is deprecated, use {$var} instead in /plugins/woocommerce-payments/includes/multi-currency/Analytics.php on line 330
  • Deprecated: Using ${var} in strings is deprecated, use {$var} instead in /plugins/woocommerce-payments/includes/multi-currency/Analytics.php on line 331
  • Deprecated: Using ${var} in strings is deprecated, use {$var} instead in /plugins/woocommerce-payments/includes/multi-currency/Analytics.php on line 332

Testing instructions:
To test this change, follow these steps:

No tests are required because the code changes involve straightforward syntax updates to resolve deprecation warnings. These changes are low-risk and don't introduce new functionality. Visual inspection and basic validation confirm the correctness of the fix. If further testing is required:

  1. Clone the repository and switch to the branch with this fix:
    git clone https://github.com/your-username/your-repo.git
    cd your-repo
    git checkout bug-fix
  2. Ensure you have the necessary development environment set up for WooCommerce Payments.
  3. Locate the Analytics.php file in the multi-currency folder within the WooCommerce Payments plugin.
  4. Check lines 330-332 and the suggested fix
  5. Identify test cases that previously triggered the deprecation warnings related to string interpolation using ${var}.
  6. Execute the identified test cases and confirm that the deprecation warnings no longer appear.
  7. Verify that the correct {$var} interpolation is now in place and that the affected code still functions as expected.

  • Run npm run changelog to add a changelog file (is patch).
  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

Post merge

@dwainm
Copy link
Contributor

dwainm commented Sep 15, 2023

Thank you for the PR @ridonibishi ! We'll take a look at it and provide any feedback as soon as possible.

Copy link
Contributor

@tpaksu tpaksu left a comment

Choose a reason for hiding this comment

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

The changes make sense, there's no need to test because the alternatives the PR provides were already in use in their previous occurrences. LGTM! And thanks for providing this fix!

@tpaksu
Copy link
Contributor

tpaksu commented Sep 15, 2023

@ridonibishi can you add a changelog before we merge this to the main repo? You can run npm run changelog after installing the necessary packages. Select patch and skip the message, add your change as a comment (the script will ask after skipping the message), since it's not a big change.

@tpaksu
Copy link
Contributor

tpaksu commented Sep 15, 2023

I took the initiative and pushed the changelog and added it to the merge queue. Thank you for catching and fixing this!

@tpaksu tpaksu enabled auto-merge September 15, 2023 17:58
@tpaksu tpaksu added this pull request to the merge queue Sep 16, 2023
Merged via the queue into Automattic:develop with commit 6ba6fab Sep 16, 2023
20 of 27 checks passed
@ridonibishi
Copy link
Contributor Author

Sorry - was on a band tour the past 4 days! Thanks for reviewing and patching! Godspeed! 💯

@ridonibishi ridonibishi deleted the bug-fix branch September 25, 2023 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants