Skip to content

refactor: [M3-8785] - Remove feature flag, tracking events used for A/B testing in API CLI tools Modal - #11156

Merged
cpathipa merged 50 commits into
linode:developfrom
cpathipa:M3-8785
Oct 29, 2024
Merged

cpathipa merged 50 commits into
linode:developfrom
cpathipa:M3-8785

Conversation

@cpathipa

Copy link
Copy Markdown
Contributor

Description 📝

This PR removes LD tracking events and feature flag used for collecting metrics for A/B testing in DX Tools Modal.

Changes 🔄

List any change relevant to the reviewer.

  • ...
  • ...

Target release date 🗓️

Please specify a release date to guarantee timely review of this PR. If exact date is not known, please approximate and update it as needed.

Verification steps

(How to verify changes)

  • Navigate to Linode create flow
  • Verify button language to "View Code Snippets"
  • Verify no regression in DX tools modal, code snippets, copy icon etc.

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

cpathipa added 30 commits June 19, 2024 09:06
@cpathipa cpathipa self-assigned this Oct 24, 2024
@cpathipa
cpathipa requested review from a team as code owners October 24, 2024 15:32
@cpathipa
cpathipa requested review from abailly-akamai, cliu-akamai and mjac0bs and removed request for a team October 24, 2024 15:32
@abailly-akamai

Copy link
Copy Markdown
Contributor

@cpathipa heads up you've got a unit failing here

@mjac0bs mjac0bs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the clean up - this is looking good minus one unneeded prop and the unit test that still needs to be update to account for the new button copy.

Comment thread packages/manager/.changeset/pr-11156-tech-stories-1729784052355.md Outdated
Comment thread packages/manager/src/components/CodeBlock/CodeBlock.tsx
Comment thread packages/manager/src/features/Linodes/LinodeCreate/Actions.tsx
@cpathipa
cpathipa requested a review from mjac0bs October 29, 2024 14:31
@github-actions

Copy link
Copy Markdown

Coverage Report: ✅
Base Coverage: 87.05%
Current Coverage: 87.06%

@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Oct 29, 2024

@abailly-akamai abailly-akamai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cleanup looks good ✅

Looking at this PR, it feels like a lot of code to cleanup for an experiment, but also it does not seem the implementation was very DRY. I think a hook would have been helpful to control the logic

@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Oct 29, 2024
@cpathipa

Copy link
Copy Markdown
Contributor Author

@abailly-akamai Good call, I will create a tech debt ticket to create a hook for firing LD tracking events to support future experiments.

@cpathipa
cpathipa merged commit 0818a7e into linode:develop Oct 29, 2024
@cpathipa
cpathipa deleted the M3-8785 branch October 29, 2024 17:42
@cypress

cypress Bot commented Oct 29, 2024

Copy link
Copy Markdown

Cloud Manager E2E    Run #6753

Run Properties:  status check passed Passed #6753  •  git commit 0818a7e6ae: refactor: [M3-8785] - Remove feature flag, tracking events used for A/B testing ...
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #6753
Run duration 26m 30s
Commit git commit 0818a7e6ae: refactor: [M3-8785] - Remove feature flag, tracking events used for A/B testing ...
Committer cpathipa
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 2
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 445
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge! Clean Up

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants