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

Jetpack App: Add 'jetpack/app-branding' flag #66814

Merged
merged 4 commits into from
Aug 22, 2022
Merged

Jetpack App: Add 'jetpack/app-branding' flag #66814

merged 4 commits into from
Aug 22, 2022

Conversation

SiobhyB
Copy link
Contributor

@SiobhyB SiobhyB commented Aug 22, 2022 •
edited
Loading

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

  • This PR introduces a jetpack/app-branding feature flag, which was added following the guidance here. The flag is enabled for development and wpcalyspo as a starting point, but doesn't change any functionality for now.

Testing Instructions

  • Verify that these changes don't introduce any unexpected errors when browsing Calypso.
  • If possible, confirm that adding the config.isEnabled('jetpack/app-branding'); works to enable/disable features in development.

Pre-merge Checklist

Add 'jetpack/app-branding' flag
896871f 
This flag will be enabled for development by default, as we begin to change mentions of the WP app to Jetpack.
@SiobhyB SiobhyB added the [Feature Group] A8C Marketing & Sales Automattic's marketing and sales operations for WordPress.com. label Aug 22, 2022
@SiobhyB SiobhyB self-assigned this Aug 22, 2022
@github-actions
Copy link

github-actions bot commented Aug 22, 2022 •
edited by matticbot
Loading

Calypso Live (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-40340
Jetpack Cloud live (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-40340&env=jetpack

@matticbot
Copy link
Contributor

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.

Siobhan added 2 commits August 22, 2022 14:53
Add flag to 'test.json'
4cda6ad
Set flag to true for 'wpcalypso.json'
dfdfc4e 
This will make it easier for developers to test the changes while working on them.
@SiobhyB SiobhyB added Mobile Application WordPress.com App for mobile Jetpack labels Aug 22, 2022
@SiobhyB
Copy link
Contributor Author

SiobhyB commented Aug 22, 2022

Noting that the E2E Tests (mobile) (Web app) failure isn't specific to this PR, it's failing for all recent PRs. See for more details: p1661153765781449/1660416713.154689-slack-C02DQP0FP

@SiobhyB SiobhyB requested a review from a team August 22, 2022 14:09
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 22, 2022
tyxla
tyxla reviewed Aug 22, 2022
View reviewed changes
config/stage.json Outdated
@@ -46,6 +46,7 @@
"inline-help": true,
"jetpack/agency-dashboard": true,
"jetpack/api-cache": true,
"jetpack/app-branding": false,
Copy link
Member

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.

Copy link
Contributor Author

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,
Copy link
Member

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?

Copy link
Contributor Author

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!

Remove flag in environments where it's not enabled
010dd41 
As flag are 'false' by default, it's okay to omit them in environments where they're not used.
@SiobhyB SiobhyB requested a review from tyxla August 22, 2022 14:27
tyxla
tyxla approved these changes Aug 22, 2022
View reviewed changes
Copy link
Member

@tyxla tyxla left a 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 💯

@SiobhyB SiobhyB marked this pull request as ready for review August 22, 2022 14:55
@SiobhyB SiobhyB merged commit 6ac475e into trunk Aug 22, 2022
@SiobhyB SiobhyB deleted the add/jetpack-app-branding-feature-flag branch August 22, 2022 16:06
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 22, 2022
@SiobhyB SiobhyB mentioned this pull request Aug 29, 2022
Merged
5 tasks
@SiobhyB SiobhyB mentioned this pull request Sep 27, 2022
Merged
5 tasks
@SiobhyB SiobhyB mentioned this pull request Oct 4, 2022
Merged
5 tasks
@SiobhyB SiobhyB mentioned this pull request Jan 18, 2023
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tyxla tyxla tyxla approved these changes

Assignees

@SiobhyB SiobhyB

Labels
[Feature Group] A8C Marketing & Sales Automattic's marketing and sales operations for WordPress.com. Jetpack Mobile Application WordPress.com App for mobile
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SiobhyB @matticbot @tyxla