-
Notifications
You must be signed in to change notification settings - Fork 1
Check if the gutenberg plugin matches the version we expect #68
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
base: trunk
Are you sure you want to change the base?
Conversation
|
At the moment, I guess it isn't fully accurate that the plugin required that version of Gutenberg - should we caution against calling out a specific version or at least mention it's a special modified version for this plugin, until we're at that point? |
My main concern with originally putting up such a PR was targeting a fix with the fork in mind, rather than the core GB plugin. It's fair that, even with 21.9.0 we would still need our special branch. But, we have to start somewhere imo. That said, I'm hoping with each subsequent release after that we can keep bumping this up as there'd be less and less in that special branch. Hopefully that answers what you meant. |
Yeah, I understand the intent, my concern was mainly around giving a not fully true message - perhaps we can link to some documentation instead that makes it clearer, and which could follow whichever is true at that point in time as we progress to having it run via official GB versions? |
I think what I have added works for now, since we are still in the beta phase. As we get closer to a full release, there'll be wpvip documentation and we'd have 100% of our changes within the GB plugin so we'd be able to truly say that x version is needed. |
|
Noting that with the release of 21.9.0 this is good to be merged in. |
|
Is there a way to do feature detection instead of version comparison? My main worry is that this gets forgotten about. |
I couldn't come up with a reliable and trusted way to do feature detection on the PHP side that'd give us the confidence to include it here. Since we are starting to make PRs upstream, I'd lean towards sticking with the version instead. Eventually, we should be able to say with confidence that if you have x version then you have everything in place to get RTC working. |
|
Maybe GUTENBERG_GIT_COMMIT until such time that we use only the trunk branch, without any special changes? That'd be specific to us. My only concern would be ensuring that we coordinate this with our RTC plugin. The current approach is a bit less riskier and easier:
This is just for the mu-plugins/integrations side tbh. |
|
What if we used the integration center API? |
|
I'm reserving that type of a check for the code that'd go into mu-plugins. I'm trying to keep this check VIP specific free because this plugin could run on non-VIP hosts. I should have mentioned that, but that's a goal I'm keeping mind. This plugin isn't meant to be VIP specific so our code shouldn't be VIP specific either. The mu-plugins code can be VIP specific, so that can take advantage of the API you linked when trying to load the integration. |
|
What if we just embed a constant or something into our fork that we can check so that we know we're always running our fork? If it's deployed through the integration center we'll know it's the latest, so we really just want to check that it's our Gutenberg and not one that has been installed from the Gutenberg release or something. |
That could work though we'd need to make sure to flag that we don't want to let that go in permanently. I am a bit hesitant because I'd like to get all our PRs into core and then drop the fork as quickly as we can imo. We would need to make sure that whenever we do switch away from that fork, we drop that check and only release Gutenberg post that release. |
I think this is a good approach and less likely than the current approach to fail via neglect. We can mark the PR in our branch as |
Co-authored-by: Chris Zarate <[email protected]>
Co-authored-by: Chris Zarate <[email protected]>
inc/Compatibility/Compatibility.php
Outdated
| wp_admin_notice( | ||
| __( | ||
| 'The Gutenberg plugin has not been installed. The VIP Real-Time Collaboration plugin has been disabled.', | ||
| 'The correct Gutenberg plugin has not been installed. The VIP Real-Time Collaboration plugin has been disabled.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I needed to communicate that the problem is that, our custom GB plugin has not been installed. Hence why I used correct here, and also avoided saying fork or VIP.
|
Note: The integration tests will pass once the Gutenberg PR is merged into our branch. |
Co-authored-by: Paul Kevan <[email protected]>

Description
We don't currently check if the Gutenberg plugin being used matches the version expected for RTC. This change verifies if the version matches what's expected which is set to 21.9.0 (RC for WordPress 6.9), and only does so if its not development mode.
It's not 100% accurate to say that 21.9.0 is what we need because there's more pieces in our special branch but we have to start somewhere. I'm hoping we can bumping this number up with each release that contains more pieces from that special branch.
Testing
Loading the normal version of Gutenberg will trigger this error because 21.9.0 hasn't been released yet.
Screenshot