-
Notifications
You must be signed in to change notification settings - Fork 21
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
Consolidate accounts onboarding: Temporarily remove Google Merchant Center and Google Ads account setup to facilitate the subsequent changes to the combo setup #2585
Consolidate accounts onboarding: Temporarily remove Google Merchant Center and Google Ads account setup to facilitate the subsequent changes to the combo setup #2585
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/2458-consolidate-google-account-cards #2585 +/- ##
===============================================================================
Coverage ? 63.8%
===============================================================================
Files ? 327
Lines ? 5090
Branches ? 1232
===============================================================================
Hits ? 3248
Misses ? 1674
Partials ? 168
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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've left one non-blocking review comment, which we can clarify with @eason9487. Otherwise, I think this looks good. Thanks @dsawardekar!
</StepContentFooter> | ||
<Section | ||
className="gla-google-mc-account-section" |
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.
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.
Agree that renaming would be more appropriate.
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.
👍🏻 Added a note in the tracking ticket to remind us to come back to this.
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.
Renamed in cc2440a.
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 update. It works as expected.
</StepContentFooter> | ||
<Section | ||
className="gla-google-mc-account-section" |
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.
Agree that renaming would be more appropriate.
465c090
into
feature/2458-consolidate-google-account-cards
…Google Merchant Center disclaimer. Address: #2585 (comment)
Changes proposed in this Pull Request:
This PR removes the cards that are being reworked from setup-accounts.
Closes #2565
Screenshots:
Detailed test instructions:
Changelog entry