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.

Sign up for GitHub

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

Jump to bottom

Embed block: Implement specific Embed preview #3725

Merged
merged 45 commits into from
Aug 11, 2021
Merged

Embed block: Implement specific Embed preview #3725

merged 45 commits into from
Aug 11, 2021

Conversation

fluiddot
Copy link
Contributor

@fluiddot fluiddot commented Jul 14, 2021 •
edited
Loading

gutenberg PR: WordPress/gutenberg#33426
WordPress-iOS PR: wordpress-mobile/WordPress-iOS#16866
WordPress-Android PR: wordpress-mobile/WordPress-Android#15051

Fixes #3276

To test:
Follow the testing instructions from gutenberg PR: WordPress/gutenberg#33426

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.

@fluiddot fluiddot self-assigned this Jul 14, 2021
This was referenced Jul 14, 2021
Merged
Merged
@fluiddot fluiddot requested a review from ceyhun July 14, 2021 15:03
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jul 20, 2021 •
edited
Loading

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

@fluiddot fluiddot mentioned this pull request Jul 21, 2021
Merged
3 tasks
@fluiddot fluiddot marked this pull request as ready for review July 30, 2021 15:31
@fluiddot fluiddot mentioned this pull request Aug 3, 2021
Closed
2 tasks
ceyhun
ceyhun approved these changes Aug 10, 2021
View reviewed changes
Copy link
Contributor

@ceyhun ceyhun left a comment

Choose a reason for hiding this comment

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

Approved via WordPress/gutenberg#33426

@fluiddot fluiddot added this to the 1.60.0 (18.1) milestone Aug 11, 2021
@fluiddot
Copy link
Contributor Author

Not sure why but the "Check Correctness" CI check is failing with error cb() never called!, I tried to restart it several times but keeps failing so I'm going to debug it just in case this PR is actually introducing the failure.

@fluiddot
Copy link
Contributor Author

fluiddot commented Aug 11, 2021 •
edited
Loading

Not sure why but the "Check Correctness" CI check is failing with error cb() never called!, I tried to restart it several times but keeps failing so I'm going to debug it just in case this PR is actually introducing the failure.

I connected to the machine instance by SSH and I verified that executing the command npm install --no-audit succeeds, running it after removing all node_modules folders also succeeds, so I have no clue regarding what it's causing the failure in this case 🤔 .

@fluiddot
Copy link
Contributor Author

Not sure why but the "Check Correctness" CI check is failing with error cb() never called!, I tried to restart it several times but keeps failing so I'm going to debug it just in case this PR is actually introducing the failure.

I connected to the machine instance by SSH and I verified that executing the command npm install --no-audit succeeds, running it after removing all node_modules folders also succeeds, so I have no clue regarding what it's causing the failure in this case 🤔 .

Finally, I managed to reproduce it by removing the folder $HOME/.npm, however, it doesn't give any clue about why fails. I'm wondering if it's related to some kind of timeout 🤔 , I'll keep investigating and let you with my findings 🔍 .

@fluiddot fluiddot merged commit bae44c3 into develop Aug 11, 2021
@fluiddot fluiddot deleted the embed-block-preview branch August 11, 2021 15:38
@fluiddot
Copy link
Contributor Author

I fixed the issue by adding a cache key fallback and re-running the jobs (it was done in this commit), this way the NPM cache has been saved for the new checksum of the Gutenberg's package-lock.json file. After this, I reverted the changes and all CI checks have passed 🎊 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ceyhun ceyhun ceyhun approved these changes

Assignees

@fluiddot fluiddot

Labels
Embed block [Type] Task
Projects
None yet
Milestone
1.60.0 (18.1)
Development

Successfully merging this pull request may close these issues.

[Embed block] Implement specific Embed preview
2 participants
@fluiddot @ceyhun