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

[Embed block] - Unavailable preview fallback bottom sheet title update #3935

Merged

Conversation

jd-alexander
Copy link
Contributor

To test:
Utilize the testing instructions at WordPress/gutenberg#34674

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Sep 9, 2021

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

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

@jd-alexander
Copy link
Contributor Author

@fluiddot I have the same node and npm version as CI. When I run npm install I and push the diff of the package-lock.json file is still fails. I am not sure what is wrong locally that could be causing this. Do you have any clues ?

@fluiddot
Copy link
Contributor

@fluiddot I have the same node and npm version as CI. When I run npm install I and push the diff of the package-lock.json file is still fails. I am not sure what is wrong locally that could be causing this. Do you have any clues ?

Not really, but I noticed that the last Dependabot's PR "Bump gutenberg from cb459f8 to e48adc4" is also failing due to a mismatch on that file:

index 39eb5f8..b79d955 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -6253,8 +6253,8 @@
 			"version": "file:gutenberg/packages/eslint-plugin",
 			"dev": true,
 			"requires": {
-				"@typescript-eslint/eslint-plugin": "^4.15.0",
-				"@typescript-eslint/parser": "^4.15.0",
+				"@typescript-eslint/eslint-plugin": "^4.31.0",
+				"@typescript-eslint/parser": "^4.31.0",
 				"@wordpress/prettier-config": "file:gutenberg/packages/prettier-config",
 				"babel-eslint": "^10.1.0",
 				"cosmiconfig": "^7.0.0",

So this might be an incoming breakage from the Gutenberg's trunk branch, I'll try it locally in case I found a workaround.

@fluiddot
Copy link
Contributor

@fluiddot I have the same node and npm version as CI. When I run npm install I and push the diff of the package-lock.json file is still fails. I am not sure what is wrong locally that could be causing this. Do you have any clues ?

Not really, but I noticed that the last Dependabot's PR "Bump gutenberg from cb459f8 to e48adc4" is also failing due to a mismatch on that file:

index 39eb5f8..b79d955 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -6253,8 +6253,8 @@
 			"version": "file:gutenberg/packages/eslint-plugin",
 			"dev": true,
 			"requires": {
-				"@typescript-eslint/eslint-plugin": "^4.15.0",
-				"@typescript-eslint/parser": "^4.15.0",
+				"@typescript-eslint/eslint-plugin": "^4.31.0",
+				"@typescript-eslint/parser": "^4.31.0",
 				"@wordpress/prettier-config": "file:gutenberg/packages/prettier-config",
 				"babel-eslint": "^10.1.0",
 				"cosmiconfig": "^7.0.0",

So this might be an incoming breakage from the Gutenberg's trunk branch, I'll try it locally in case I found a workaround.

I've just noticed that you fixed this in commit 4b66b80 so I'm not sure yet what's causing this 🤔 .

@fluiddot
Copy link
Contributor

I managed to fix it by running the command npm install @typescript-eslint/eslint-plugin@latest --save-dev as it's mentioned in the error message:

ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin".

(The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/home/circleci/project".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

    npm install @typescript-eslint/eslint-plugin@latest --save-dev

The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in ".eslintrc.js » plugin:@wordpress/recommended".

However, I'm not sure why we need this is needed now if that package was referenced weeks ago (reference) 🤔 .

@fluiddot
Copy link
Contributor

I managed to fix it by running the command npm install @typescript-eslint/eslint-plugin@latest --save-dev as it's mentioned in the error message

I applied this fix in "Bump gutenberg from cb459f8 to e48adc4" PR and the "Check Correctness" job passed 🟢 .

@fluiddot
Copy link
Contributor

However, I'm not sure why we need this is needed now if that package was referenced weeks ago (reference) 🤔 .

Ok, looks like it's caused by the modification of the package-lock.json file, here you can see that the @typescript-eslint/eslint-plugin root-level dependency is removed. Nevertheless, it's still unclear to me why it gets removed 🤔 .

@jd-alexander
Copy link
Contributor Author

Ok, looks like it's caused by the modification of the package-lock.json file, here you can see that the @typescript-eslint/eslint-plugin root-level dependency is removed. Nevertheless, it's still unclear to me why it gets removed 🤔 .

Ah, understood! I will sync with develop once #3955 is merged in. 🙏🏾

@fluiddot
Copy link
Contributor

Ok, looks like it's caused by the modification of the package-lock.json file, here you can see that the @typescript-eslint/eslint-plugin root-level dependency is removed. Nevertheless, it's still unclear to me why it gets removed 🤔 .

Ah, understood! I will sync with develop once #3955 is merged in. 🙏🏾

I've just merged #3955 into develop so feel free to sync it.

@jd-alexander jd-alexander force-pushed the embed-block-unavailable-preview-fallback-text-fix branch from 5869390 to 9c40147 Compare September 14, 2021 21:54
@jd-alexander jd-alexander merged commit 2b85251 into develop Sep 14, 2021
@jd-alexander jd-alexander deleted the embed-block-unavailable-preview-fallback-text-fix branch September 14, 2021 23:08
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.

2 participants