Gutenboarding: Remove all unused bits and reorganize the rest #74475
Conversation
This file contains 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
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~375546 bytes removed 📉 [gzipped]) Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~19467 bytes removed 📉 [gzipped]) Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Async-loaded Components (~60613 bytes removed 📉 [gzipped]) React components that are loaded lazily, when a certain part of UI is displayed for the first time. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
tyxla
changed the title
This was referenced Mar 15, 2023
tyxla
added
Framework
[Type] Janitorial
[Goal] New Onboarding
matticbot
added
the
[Status] Needs Review
7 tasks
noahtallen
reviewed
Mar 16, 2023
| @@ -24,3 +25,5 @@ export const USER_STORE = User.register( { | |||
| } ); | |||
|
|
|||
| export const AUTOMATED_ELIGIBILITY_STORE = AutomatedTransferEligibility.register(); | |||
|
|
|||
| export const PLANS_STORE = Plans.register(); | |||
There was a problem hiding this comment.
Good example of how importing directly from the actual store is useful! This change probably wouldn't have been needed if the code was already using the new redux pattern :)
noahtallen
approved these changes
Mar 16, 2023
tyxla
force-pushed
the
remove/gutenboarding
branch
from
a2cd29e to
fb9fc7a
Compare
worldomonation
approved these changes
Mar 16, 2023
There was a problem hiding this comment.
I know I'm not part of the reviewers list, but wanted to say thank you for going back and removing deprecated code. It's an underappreciated task.
I do not believe any E2E tests remain that uses Gutenboarding, but POMs do remain. I will create another PR based on this branch that cleans those POMs up.
sirbrillig
approved these changes
Mar 16, 2023
alshakero
approved these changes
Mar 16, 2023
Comment on lines
+47
to
+67
| /* | ||
| * Parses the `anchor_podcast` parameter from a given URL. | ||
| * Returns `true` if provided URL is an anchor FM signup URL. | ||
| */ | ||
| function getIsAnchorFmSignup( urlString ) { | ||
| if ( ! urlString ) { | ||
| return false; | ||
| } | ||
|
|
||
| // Assemble search params if there is actually a query in the string. | ||
| const queryParamIndex = urlString.indexOf( '?' ); | ||
| if ( queryParamIndex === -1 ) { | ||
| return false; | ||
| } | ||
| const searchParams = new URLSearchParams( | ||
| decodeURIComponent( urlString.slice( queryParamIndex ) ) | ||
| ); | ||
| const anchorFmPodcastId = searchParams.get( 'anchor_podcast' ); | ||
| return Boolean( anchorFmPodcastId && anchorFmPodcastId.match( /^[0-9a-f]{7,8}$/i ) ); | ||
| } | ||
|
|
There was a problem hiding this comment.
We have another one of these here. Maybe worth putting in the onboarding package.
There was a problem hiding this comment.
Given that those are different functions, I preferred not to introduce additional dependencies to the Login block used in multiple places, but this is definitely something worth arranging at some point.
|
Not gonna lie, having spent months working on this, it breaks my heart a little. |
|
Thank you all for taking a look 🙌
I totally sympathize. It served us really well! |
github-actions
bot
removed
the
[Status] Needs Review
This was referenced Apr 4, 2023
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While working on #74399 I noticed that none of Gutenboarding is accessible, since it would always redirect to
/start:wp-calypso/client/server/pages/index.js
Lines 901 to 915 in 3626bab
Regardless that we won't be using it, we are building the Gutenboarding entry point along with all of its code.
Might be easier to review on a per-commit basis, since all changes that aren't removals are in separate commits.
Proposed Changes
This PR removes the unused Gutenboarding entry point, along with all unused code from there.
The little remaining code is being moved where it is currently used and makes more sense.
Testing Instructions
/new*)