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

chore: add builder for keeping track of references found #76

Merged
merged 11 commits into from
Nov 3, 2023

Conversation

jazanne
Copy link
Contributor

@jazanne jazanne commented Nov 3, 2023

Moves a lot logic out of ProcessDiffs by using a builder to track references.

This allows us to use Build() to do additional processing that other functions were doing (like deduping added/removed flags as "modified").

This sets us up to handle monorepo support a lot more easily.

@jazanne jazanne changed the title Jwhite/ref builder chore: add builder for keeping track of references found Nov 3, 2023
@jazanne jazanne marked this pull request as ready for review November 3, 2023 20:03
@jazanne jazanne requested a review from a team November 3, 2023 20:03
Comment on lines -134 to -135
// If flag is in both added and removed then it is being modified
delete(flagsRef.FlagsRemoved, flagKey)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

deduping "modified" flags now happens in Build()

@@ -131,9 +131,7 @@ func ProcessFlags(flagsRef lflags.FlagsRef, flags []ldapi.FeatureFlag, config *l
// sort keys so hashing can work for checking if comment already exists
sort.Strings(addedKeys)
for _, flagKey := range addedKeys {
// If flag is in both added and removed then it is being modified
delete(flagsRef.FlagsRemoved, flagKey)
flagAliases := uniqueAliases(flagsRef.FlagsAdded[flagKey])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove duplicate or empty alias strings is now handled by Build()

Copy link

github-actions bot commented Nov 3, 2023

LaunchDarkly flag references

🔍 2 flags added or modified

Name Key Aliases found
Beta UI beta-ui betaUi
old-pricing-banner old-pricing-banner oldPricingBanner

❌ 1 flag removed

Name Key Aliases found
Show: Live Tiles show-widgets show_widgets

@github-actions github-actions bot added the ld-flags LaunchDarkly flags have been detected in the PR diff label Nov 3, 2023
flagMap[op][flagKey][alias] = true
}
}
aliasMatches := elementMatcher.FindAliases(line, flagKey)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was always supposed to be calling elementMatcher.FindAliases - since we don't have monorepo support, it hasn't mattered so far that it wasn't

Copy link

@ld-kyee ld-kyee left a comment

Choose a reason for hiding this comment

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

🚢

flags/builder.go Outdated Show resolved Hide resolved
diff/diff.go Outdated Show resolved Hide resolved
@jazanne jazanne merged commit 98ace1f into main Nov 3, 2023
5 checks passed
@jazanne jazanne deleted the jwhite/ref-builder branch November 3, 2023 20:42
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.

2 participants