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

Remove React 17 requirement for AfterActivationSiteScan service #7490

Merged
merged 1 commit into from
Mar 16, 2023

Conversation

thelovekesh
Copy link
Collaborator

@thelovekesh thelovekesh commented Mar 16, 2023

Summary

Fixes #7489

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@thelovekesh thelovekesh marked this pull request as ready for review March 16, 2023 19:25
@github-actions
Copy link
Contributor

github-actions bot commented Mar 16, 2023

Plugin builds for ba02543 are ready 🛎️!

- Why? React 17 code will continue to work with React 18 by invoking a legacy API
@thelovekesh thelovekesh changed the title Revert #7421 Remove React 17 requirement for AfterActivationSiteScan service Mar 16, 2023
@@ -23,9 +23,6 @@ cd "$(dirname "$0")/../.."
# done on every time `npm run test-e2e` is run.
. "$(dirname "$0")/install-wordpress.sh"

# Install specific version of Gutenberg plugin for e2e tests.
download_gutenberg "14.7.3" "tests/e2e/plugins/gutenberg.14.7.3.zip"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just in case we ever need it, should we leave the download_gutenberg util function in place?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same for uploads.ini

- ./uploads.ini:/usr/local/etc/php/conf.d/uploads.ini

Copy link
Member

Choose a reason for hiding this comment

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

We could, but probably better to remove it. It can be resurrected via version control if needed.

@thelovekesh
Copy link
Collaborator Author

Looks like flakiness in e2e is still there, even after #7488.

@westonruter westonruter added this to the v2.4.1 milestone Mar 16, 2023
Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

LGTM

@westonruter westonruter merged commit 4337065 into develop Mar 16, 2023
@westonruter westonruter deleted the revert/7421 branch March 16, 2023 20:29
@pavanpatil1
Copy link

QA Passed ✅

Verified the issue, the site scan function is working fine with the React 18 and WP6.2 current RC version.

image

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove React 17 requirement for certain features
3 participants