FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Implement migration prompt to migrate from SQLite module to standalone plugin due to removal in the following release by felixarntz · Pull Request #739 · WordPress/performance · GitHub

/ performance Public

Implement migration prompt to migrate from SQLite module to standalone plugin due to removal in the following release - #739

Merged
felixarntz merged 3 commits into
trunkfrom
add/737-sqlite-removal-notice
Jun 6, 2023
ConversationCommits (3)ChecksFiles changed
Merged

Implement migration prompt to migrate from SQLite module to standalone plugin due to removal in the following release#739
felixarntz merged 3 commits into
trunkfrom
add/737-sqlite-removal-notice

Conversation

felixarntz commented May 31, 2023

Copy link
Copy Markdown
Member

Summary

Fixes #737

Relevant technical choices

  • An admin notice is added with 3 steps to migrate, each of which is conditionally marked already completed depending where the site owner is at in the process.
  • An admin pointer is added as an additional way to make site owners aware that they need to take an action, since admin notices can easily be overlooked across other admin notices.
  • A few minor interoperability fixes we added (in the modules/database/sqlite files).
  • No new functions are being introduced since they would need to be removed again in the following release. Therefore the new temporary code relies heavily on closures.
  • Also see the requirements on the issue.

Screenshots

Admin notice awaiting step 1

Admin notice awaiting step 2

Admin notice awaiting step 3

Admin pointer

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

felixarntz added [Type] Enhancement A suggestion for improvement of an existing feature [Focus] Database labels May 31, 2023
felixarntz added this to the 2.4.0 milestone May 31, 2023
mukeshpanchal27 reviewed Jun 1, 2023
View reviewed changes

mukeshpanchal27 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks, @felixarntz. It looks good to me. I have some nitpick feedback and a few questions.

Sometimes, when we activate the SQLite Database Integration through step 2 and then return to the Performance Lab settings page, deactivating the module also deactivates the SQLite Database Integration plugin. is it intended behaviour?

Comment thread admin/load.php
Comment thread admin/load.php

felixarntz commented Jun 1, 2023

Copy link
Copy Markdown
Member Author

@mukeshpanchal27

Sometimes, when we activate the SQLite Database Integration through step 2 and then return to the Performance Lab settings page, deactivating the module also deactivates the SQLite Database Integration plugin. is it intended behaviour?

I noticed that too, that's okay. It is due to the migration routine that the standalone plugin comes with. In the cases where the plugin takes care of that automatically, even better, one less step for the user.

mukeshpanchal27 approved these changes Jun 2, 2023
View reviewed changes

mukeshpanchal27 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks @felixarntz, LGTM!

felixarntz commented Jun 5, 2023

Copy link
Copy Markdown
Member Author

@aristath @joemcgill Would one of you be able to review this PR please?

aristath approved these changes Jun 6, 2023
View reviewed changes

aristath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM! 👍

felixarntz merged commit 149d582 into trunk Jun 6, 2023
felixarntz deleted the add/737-sqlite-removal-notice branch June 6, 2023 15:26
felixarntz mentioned this pull request Jun 16, 2023
Merged
3 tasks
mukeshpanchal27 mentioned this pull request Dec 18, 2023
Merged
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

aristath aristath approved these changes
mukeshpanchal27 mukeshpanchal27 approved these changes
OllieJones Awaiting requested review from OllieJones
JustinyAhin Awaiting requested review from JustinyAhin
joemcgill Awaiting requested review from joemcgill

Assignees

No one assigned

Labels

[Type] Enhancement A suggestion for improvement of an existing feature

Projects

None yet

Milestone

2.4.0

Development

Successfully merging this pull request may close these issues.

Implement notice to prompt administrator to update to SQLite plugin instead of SQLite module

3 participants

Back | FazBrowse Home | New Git URL