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.

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

feat: detect flag extinctions #73

Merged
merged 35 commits into from
Nov 13, 2023
Merged

feat: detect flag extinctions #73

merged 35 commits into from
Nov 13, 2023

Conversation

jazanne
Copy link
Contributor

@jazanne jazanne commented Nov 2, 2023

For each removed flag, we will check if there are any other references remaining i the code! This will make cleanup much easier

Learn more about extinct flags

Open to see design options ## LaunchDarkly flag references ### ❌ 1 flag removed

Table options:

Option 1:

Name Key Aliases found
Show widgets show-widgets showWidgets
💀 Beta UI beta-ui betaUi, beta_ui

Option 2:

Name Key Aliases found
Show widgets show-widgets showWidgets
Beta UI (no remaining references) beta-ui betaUi, beta_ui

Option 3:

Name Key Aliases found All references removed
Show widgets show-widgets showWidgets false
Beta UI beta-ui betaUi, beta_ui true

Option 4:

Name Key Aliases found All references removed
Show widgets show-widgets showWidgets
Beta UI beta-ui betaUi, beta_ui

Copy link

github-actions bot commented Nov 2, 2023

LaunchDarkly flag references

🔍 2 flags added or modified

Name Key Aliases found Info
Show: App Promo mobile-app-promo-ios mobile-app-promo-ios ⚠️ archived on 2023-08-03
Show: Live Tiles show-widgets show_widgets

❌ 1 flag removed

Name Key Aliases found Info
Beta UI beta-ui betaUi, beta_ui ✅ all references removed
ℹ️ archived on 2023-11-13

@github-actions github-actions bot added the ld-flags LaunchDarkly flags have been detected in the PR diff label Nov 2, 2023
@jazanne jazanne changed the title detect extinctions feat: detect flag extinctions Nov 2, 2023
@ld-kyee
Copy link

ld-kyee commented Nov 2, 2023

I think this is great! Although, I'm not sure the skull is very clear. Maybe we need another column that says "all references removed" or something like this.

@UPatel12
Copy link

UPatel12 commented Nov 3, 2023

confirming that if there are no remaining references then the alias found colmn should be empty, right?

@jazanne
Copy link
Contributor Author

jazanne commented Nov 3, 2023

confirming that if there are no remaining references then the alias found colmn should be empty, right?

@UPatel12 No, the alias found column is showing that those aliases were found and removed - will tag you in a comment to make it more clear

@@ -1,11 +1,7 @@
show-widgets
showWidgets
betaUi
Copy link
Contributor Author

@jazanne jazanne Nov 3, 2023

Choose a reason for hiding this comment

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

@UPatel12 this alias of beta-ui is what was removed, so we try to let people know

@UPatel12
Copy link

UPatel12 commented Nov 3, 2023

I'm leaning toward option 2 (without the check mark) out of the two options. The skull & check mark are a little confusing.

I like Kerrie's idea of adding a new column for all references that have been removed.

2 options:

  • Column can be True or False. Are all references removed or not.
  • Column can be a count of the references found/remaining. For extinct flags, the colmn entry would be zero.

@jazanne
Copy link
Contributor Author

jazanne commented Nov 3, 2023

Column can be a count of the references found/remaining. For extinct flags, the colmn entry would be zero.

we don't scan for all flags in repo so we can't do count

I really think a boolean column w/true or false will not be visually compelling enough

Name Key Aliases found All references removed
Show widgets show-widgets showWidgets false
Beta UI beta-ui betaUi, beta_ui true

Maybe the column is something like "all references removed" and then has a checkmark

Name Key Aliases found All references removed
Show widgets show-widgets showWidgets
Beta UI beta-ui betaUi, beta_ui

I want to make sure we're presenting all the info in a way that's really easy to grasp from a quick scan.

@jazanne
Copy link
Contributor Author

jazanne commented Nov 13, 2023

Proposal for change to 4 columns...

LaunchDarkly flag references

🔍 2 flags added or modified

Flag name Key Aliases found Info
Show: Live Tiles show-widgets showWidgets
old-pricing-banner old-pricing-banner old-pricing-banner ⚠️ archived on 2023-08-03

❌ 1 flag removed

Flag name Key Aliases found Info
Show: App Promo mobile-app-promo-ios mobileAppPromoIos ℹ️ archived on 2023-08-03
Beta UI beta-ui betaUi, beta_ui ✅ all references removed

@jazanne jazanne marked this pull request as ready for review November 13, 2023 20:08
@jazanne jazanne requested a review from a team November 13, 2023 20:08
}
setOutputsForChangedFlags("removed", flagsModified)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

sad, this was wrong before

@jazanne jazanne merged commit a2d15e1 into main Nov 13, 2023
5 checks passed
@jazanne jazanne deleted the jwhite/extinctions branch November 13, 2023 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ld-flags LaunchDarkly flags have been detected in the PR diff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants