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

Suppress Site Health ICU test if site or home URL is not an IDN #4698

Merged
merged 5 commits into from
May 14, 2020

Conversation

pierlon
Copy link
Contributor

@pierlon pierlon commented May 13, 2020

Summary

Fixes #4616.

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • 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).

@googlebot googlebot added the cla: yes Signed the Google CLA label May 13, 2020
tests/php/test-class-site-health.php Outdated Show resolved Hide resolved
src/Admin/SiteHealth.php Outdated Show resolved Hide resolved
src/Admin/SiteHealth.php Outdated Show resolved Hide resolved
src/Admin/SiteHealth.php Outdated Show resolved Hide resolved
Make `is_intl_extension_needed()` private

Co-authored-by: Weston Ruter <westonruter@google.com>
@pierlon
Copy link
Contributor Author

pierlon commented May 13, 2020

Looks like Travis is broken, merging the develop branch to give it a push.

@westonruter
Copy link
Member

Something seems off here:

image

Why is there no merge commit?

@pierlon
Copy link
Contributor Author

pierlon commented May 13, 2020

Made an oopsie there, forgot to pull before merging, reverting those commits and pushing the merge commit.

@pierlon
Copy link
Contributor Author

pierlon commented May 13, 2020

Some Travis jobs are still running so I'll wait for those to complete first.

@pierlon pierlon force-pushed the enhancement/4616-hide-icu-site-health-test branch from b4bc5e5 to d6f0123 Compare May 13, 2020 22:24
@westonruter westonruter merged commit 2da4b55 into develop May 14, 2020
@westonruter westonruter deleted the enhancement/4616-hide-icu-site-health-test branch May 14, 2020 01:34
westonruter added a commit that referenced this pull request May 14, 2020
* Suppress ICU test if site or home URL is not an IDN

* Check if any domain labels start with `xn--` to determine if a domain is an IDN

* Return true if IDN domain found

* Update src/Admin/SiteHealth.php

Make `is_intl_extension_needed()` private

Co-authored-by: Weston Ruter <westonruter@google.com>

Co-authored-by: Weston Ruter <westonruter@google.com>
westonruter added a commit that referenced this pull request May 15, 2020
…ve-collection-schema-type

* 'develop' of github.com:ampproject/amp-wp:
  Raise PHPStan level to 4 in lib/common (#4686)
  Update dependency uuid to v8
  Update dependency webpack to v4.43.0
  Update dependency eslint-plugin-jest to v23.11.0
  Update dependency eslint-plugin-react to v7.20.0 (#4691)
  Update dependency postcss to v7.0.30 (#4649)
  Update dependency moment to v2.25.3 (#4639)
  Update dependency babel-jest to v26 (#4652)
  Update dependency uuid to v7.0.3 (#4490)
  Update dependency wp-coding-standards/wpcs to v2.3.0 (#4721)
  Suppress Site Health ICU test if site or home URL is not an IDN (#4698)
  Pin amp-experiment to v0.1 (#4690)
  Strip multiple BOM characters (#4683)
  Strip leading BOM and whitespace and trailing HTML comment before parsing validation response JSON (#4679)
  Disable share icons
  Use amp-embedly-card for Tiktok embeds
  Update dependency xwp/wp-dev-lib to v1.6.4
  Update dependency eslint to v7
  Update dependency terser-webpack-plugin to v2.3.6
westonruter added a commit that referenced this pull request May 15, 2020
…-phpstan-to-level-4

* 'develop' of github.com:ampproject/amp-wp:
  Update dependency @wordpress/scripts to v9.1.0
  Update dependency @wordpress/jest-puppeteer-axe to v1.8.0
  Update dependency @wordpress/url to v2.15.0
  Update dependency @wordpress/plugins to v2.16.0
  Raise PHPStan level to 4 in lib/common (#4686)
  Update dependency uuid to v8
  Update dependency webpack to v4.43.0
  Update dependency eslint-plugin-jest to v23.11.0
  Update dependency eslint-plugin-react to v7.20.0 (#4691)
  Update dependency postcss to v7.0.30 (#4649)
  Update dependency moment to v2.25.3 (#4639)
  Update dependency babel-jest to v26 (#4652)
  Update dependency uuid to v7.0.3 (#4490)
  Update dependency wp-coding-standards/wpcs to v2.3.0 (#4721)
  Update dependency @wordpress/e2e-test-utils to v4.7.0
  Suppress Site Health ICU test if site or home URL is not an IDN (#4698)
  Pin amp-experiment to v0.1 (#4690)
  Disable share icons
  Use amp-embedly-card for Tiktok embeds
  Update dependency terser-webpack-plugin to v2.3.6
@amedina
Copy link
Member

amedina commented May 18, 2020

@pierlon Please specify QA/Testing instructions for this PR/issue.

@pierlon
Copy link
Contributor Author

pierlon commented May 18, 2020

@amedina I've updated the addressed issue with testing instructions.

@westonruter westonruter added this to the v1.5.4 milestone Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Signed the Google CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Site Health test for ICU should be suppressed if site/home URLs are not in IDN (punycode)
4 participants