-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Jetpack App: Add 'jetpack/app-branding' flag #66814
Conversation
This flag will be enabled for development by default, as we begin to change mentions of the WP app to Jetpack.
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
This will make it easier for developers to test the changes while working on them.
Noting that the |
config/stage.json
Outdated
@@ -46,6 +46,7 @@ | |||
"inline-help": true, | |||
"jetpack/agency-dashboard": true, | |||
"jetpack/api-cache": true, | |||
"jetpack/app-branding": false, |
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.
Feel free to omit the flag in environment configurations when it will be disabled. Feature flags are disabled by default, so if we omit it, it works the same way as if it's specifically 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.
Thanks for the heads up! I've gone ahead to remove the flag from environments it's not used in 010dd41. 🙇♀️
config/test.json
Outdated
@@ -44,6 +44,7 @@ | |||
"importers/substack": true, | |||
"inline-help": true, | |||
"jetpack/agency-dashboard": true, | |||
"jetpack/app-branding": true, |
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.
Do we need it enabled for the testing environment as well?
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.
No, that was a mistake on my side. Removed as part of 010dd41. Thanks!
As flag are 'false' by default, it's okay to omit them in environments where they're not used.
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.
This looks great 👍 Excited to see the work on the WP.com functionality be moved to the Jetpack app 💯
Following the plans to refocus the WordPress apps on core features, we plan to move features that are specific to WordPress.com to the Jetpack app. As such, we will eventually replace all mentions of the WordPress app in Calypso with Jetpack. Making these changes behind a feature flag for now will enable us to make progress and ship all necessary changes to Calypso when ready.
Proposed Changes
jetpack/app-branding
feature flag, which was added following the guidance here. The flag is enabled fordevelopment
andwpcalyspo
as a starting point, but doesn't change any functionality for now.Testing Instructions
config.isEnabled('jetpack/app-branding');
works to enable/disable features in development.Pre-merge Checklist