Skip to content

Commit

Permalink
chore: refactor finding flag and alias matches (#31)
Browse files Browse the repository at this point in the history 
Main changes are to `ProcessDiff` function which looks for flag matches
and alias matches. It's a small-ish refactor to make the function easier
to understand.

The rest of the changes are type-related to better support the refactor
of `ProcessDiffs`
  • Loading branch information
@jazanne
jazanne authored Jun 30, 2023
1 parent 36c09b5 commit 59d4bf1
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 122 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/main.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,9 @@ jobs:
for flag in ${{ steps.find_flags.outputs.removed-flags }}; do
echo "$flag was removed"
done
- name: Add label
if: steps.find_flags.outputs.any-modified == 'true' || steps.find_flags.outputs.any-removed == 'true'
run: gh pr edit $PR_NUMBER --add-label ld-flags
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
PR_NUMBER: ${{ github.event.pull_request.number }}
46 changes: 18 additions & 28 deletions comments/comments.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
ldapi "github.com/launchdarkly/api-client-go/v7"
"github.com/launchdarkly/cr-flags/config"
lcr "github.com/launchdarkly/cr-flags/config"
lflags "github.com/launchdarkly/cr-flags/flags"
)

type Comment struct {
Expand Down Expand Up @@ -74,16 +75,7 @@ type FlagComments struct {
CommentsRemoved []string
}

type FlagsRef struct {
FlagsAdded map[string][]string
FlagsRemoved map[string][]string
}

func (fr FlagsRef) Found() bool {
return len(fr.FlagsAdded) > 0 || len(fr.FlagsRemoved) > 0
}

func BuildFlagComment(buildComment FlagComments, flagsRef FlagsRef, existingComment *github.IssueComment) string {
func BuildFlagComment(buildComment FlagComments, flagsRef lflags.FlagsRef, existingComment *github.IssueComment) string {
tableHeader := "| Flag name | Key | Aliases |\n| --- | --- | --- |"

var commentStr []string
Expand All @@ -103,8 +95,9 @@ func BuildFlagComment(buildComment FlagComments, flagsRef FlagsRef, existingComm
commentStr = append(commentStr, tableHeader)
commentStr = append(commentStr, buildComment.CommentsRemoved...)
}
allFlagKeys := uniqueKeys(flagsRef.FlagsAdded, flagsRef.FlagsRemoved)
allFlagKeys := uniqueFlagKeys(flagsRef.FlagsAdded, flagsRef.FlagsRemoved)
if len(allFlagKeys) > 0 {
sort.Strings(allFlagKeys)
commentStr = append(commentStr, fmt.Sprintf(" <!-- flags:%s -->", strings.Join(allFlagKeys, ",")))
}
postedComments := strings.Join(commentStr, "\n")
Expand All @@ -119,7 +112,7 @@ func BuildFlagComment(buildComment FlagComments, flagsRef FlagsRef, existingComm
return postedComments
}

func ProcessFlags(flagsRef FlagsRef, flags ldapi.FeatureFlags, config *lcr.Config) FlagComments {
func ProcessFlags(flagsRef lflags.FlagsRef, flags ldapi.FeatureFlags, config *lcr.Config) FlagComments {
buildComment := FlagComments{}
addedKeys := make([]string, 0, len(flagsRef.FlagsAdded))
for key := range flagsRef.FlagsAdded {
Expand All @@ -130,14 +123,7 @@ func ProcessFlags(flagsRef FlagsRef, flags ldapi.FeatureFlags, config *lcr.Confi
for _, flagKey := range addedKeys {
// If flag is in both added and removed then it is being modified
delete(flagsRef.FlagsRemoved, flagKey)
aliases := flagsRef.FlagsAdded[flagKey]

flagAliases := aliases[:0]
for _, alias := range aliases {
if !(len(strings.TrimSpace(alias)) == 0) {
flagAliases = append(flagAliases, alias)
}
}
flagAliases := uniqueAliases(flagsRef.FlagsAdded[flagKey])
idx, _ := find(flags.Items, flagKey)
createComment, err := githubFlagComment(flags.Items[idx], flagAliases, config)
buildComment.CommentsAdded = append(buildComment.CommentsAdded, createComment)
Expand All @@ -151,13 +137,7 @@ func ProcessFlags(flagsRef FlagsRef, flags ldapi.FeatureFlags, config *lcr.Confi
}
sort.Strings(removedKeys)
for _, flagKey := range removedKeys {
aliases := flagsRef.FlagsRemoved[flagKey]
flagAliases := aliases[:0]
for _, alias := range aliases {
if !(len(strings.TrimSpace(alias)) == 0) {
flagAliases = append(flagAliases, alias)
}
}
flagAliases := uniqueAliases(flagsRef.FlagsRemoved[flagKey])
idx, _ := find(flags.Items, flagKey)
removedComment, err := githubFlagComment(flags.Items[idx], flagAliases, config)
buildComment.CommentsRemoved = append(buildComment.CommentsRemoved, removedComment)
Expand All @@ -178,7 +158,7 @@ func find(slice []ldapi.FeatureFlag, val string) (int, bool) {
return -1, false
}

func uniqueKeys(a map[string][]string, b map[string][]string) []string {
func uniqueFlagKeys(a, b lflags.FlagAliasMap) []string {
maxKeys := len(a) + len(b)
allKeys := make([]string, 0, maxKeys)
for k := range a {
Expand All @@ -194,6 +174,16 @@ func uniqueKeys(a map[string][]string, b map[string][]string) []string {
return allKeys
}

func uniqueAliases(aliases lflags.AliasSet) []string {
flagAliases := make([]string, 0, len(aliases))
for alias := range aliases {
if len(strings.TrimSpace(alias)) > 0 {
flagAliases = append(flagAliases, alias)
}
}
return flagAliases
}

func pluralize(str string, strLength int) string {
tmpl := "%d %s"
if strLength != 1 {
Expand Down
39 changes: 20 additions & 19 deletions comments/comments_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

ldapi "github.com/launchdarkly/api-client-go/v7"
"github.com/launchdarkly/cr-flags/config"
lflags "github.com/launchdarkly/cr-flags/flags"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -56,17 +57,17 @@ func createFlag(key string) ldapi.FeatureFlag {

type testCommentBuilder struct {
Comments FlagComments
FlagsRef FlagsRef
FlagsRef lflags.FlagsRef
}

func newCommentBuilderAccEnv() *testCommentBuilder {
flagComments := FlagComments{
CommentsAdded: []string{},
CommentsRemoved: []string{},
}
flagsAdded := make(map[string][]string)
flagsRemoved := make(map[string][]string)
flagsRef := FlagsRef{
flagsAdded := make(lflags.FlagAliasMap)
flagsRemoved := make(lflags.FlagAliasMap)
flagsRef := lflags.FlagsRef{
FlagsAdded: flagsAdded,
FlagsRemoved: flagsRemoved,
}
Expand All @@ -79,17 +80,17 @@ func newCommentBuilderAccEnv() *testCommentBuilder {

type testProcessor struct {
Flags ldapi.FeatureFlags
FlagsRef FlagsRef
FlagsRef lflags.FlagsRef
Config config.Config
}

func newProcessFlagAccEnv() *testProcessor {
flag := createFlag("example-flag")
flags := ldapi.FeatureFlags{}
flags.Items = append(flags.Items, flag)
flagsAdded := make(map[string][]string)
flagsRemoved := make(map[string][]string)
flagsRef := FlagsRef{
flagsAdded := make(lflags.FlagAliasMap)
flagsRemoved := make(lflags.FlagAliasMap)
flagsRef := lflags.FlagsRef{
FlagsAdded: flagsAdded,
FlagsRemoved: flagsRemoved,
}
Expand All @@ -109,9 +110,9 @@ func newProcessMultipleFlagsFlagAccEnv() *testProcessor {
flag := createFlag("example-flag")
flag2 := createFlag("second-flag")
flags := ldapi.FeatureFlags{Items: []ldapi.FeatureFlag{flag, flag2}}
flagsAdded := make(map[string][]string)
flagsRemoved := make(map[string][]string)
flagsRef := FlagsRef{
flagsAdded := make(lflags.FlagAliasMap)
flagsRemoved := make(lflags.FlagAliasMap)
flagsRef := lflags.FlagsRef{
FlagsAdded: flagsAdded,
FlagsRemoved: flagsRemoved,
}
Expand Down Expand Up @@ -174,7 +175,7 @@ func (e *testFlagEnv) Alias(t *testing.T) {
}

func (e *testCommentBuilder) AddedOnly(t *testing.T) {
e.FlagsRef.FlagsAdded["example-flag"] = []string{}
e.FlagsRef.FlagsAdded["example-flag"] = lflags.AliasSet{}
e.Comments.CommentsAdded = []string{"comment1", "comment2"}
comment := BuildFlagComment(e.Comments, e.FlagsRef, nil)

Expand All @@ -183,8 +184,8 @@ func (e *testCommentBuilder) AddedOnly(t *testing.T) {
}

func (e *testCommentBuilder) RemovedOnly(t *testing.T) {
e.FlagsRef.FlagsRemoved["example-flag"] = []string{}
e.FlagsRef.FlagsRemoved["sample-flag"] = []string{}
e.FlagsRef.FlagsRemoved["example-flag"] = lflags.AliasSet{}
e.FlagsRef.FlagsRemoved["sample-flag"] = lflags.AliasSet{}
e.Comments.CommentsRemoved = []string{"comment1", "comment2"}
comment := BuildFlagComment(e.Comments, e.FlagsRef, nil)

Expand All @@ -193,8 +194,8 @@ func (e *testCommentBuilder) RemovedOnly(t *testing.T) {
}

func (e *testCommentBuilder) AddedAndRemoved(t *testing.T) {
e.FlagsRef.FlagsAdded["example-flag"] = []string{}
e.FlagsRef.FlagsRemoved["example-flag"] = []string{}
e.FlagsRef.FlagsAdded["example-flag"] = lflags.AliasSet{}
e.FlagsRef.FlagsRemoved["example-flag"] = lflags.AliasSet{}
e.Comments.CommentsAdded = []string{"comment1", "comment2"}
e.Comments.CommentsRemoved = []string{"comment1", "comment2"}
comment := BuildFlagComment(e.Comments, e.FlagsRef, nil)
Expand All @@ -206,7 +207,7 @@ func (e *testCommentBuilder) AddedAndRemoved(t *testing.T) {
}

func (e *testProcessor) Basic(t *testing.T) {
e.FlagsRef.FlagsAdded["example-flag"] = []string{""}
e.FlagsRef.FlagsAdded["example-flag"] = lflags.AliasSet{"": true}
processor := ProcessFlags(e.FlagsRef, e.Flags, &e.Config)
expected := FlagComments{
CommentsAdded: []string{"| [example flag](https://example.com/test) | `example-flag` | |"},
Expand All @@ -215,8 +216,8 @@ func (e *testProcessor) Basic(t *testing.T) {
}

func (e *testProcessor) Multi(t *testing.T) {
e.FlagsRef.FlagsAdded["example-flag"] = []string{""}
e.FlagsRef.FlagsAdded["second-flag"] = []string{""}
e.FlagsRef.FlagsAdded["example-flag"] = lflags.AliasSet{"": true}
e.FlagsRef.FlagsAdded["second-flag"] = lflags.AliasSet{"": true}
processor := ProcessFlags(e.FlagsRef, e.Flags, &e.Config)
expected := FlagComments{
CommentsAdded: []string{
Expand Down
94 changes: 49 additions & 45 deletions diff/diff.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"strings"

ldapi "github.com/launchdarkly/api-client-go/v7"
ghc "github.com/launchdarkly/cr-flags/comments"
lflags "github.com/launchdarkly/cr-flags/flags"
"github.com/launchdarkly/cr-flags/ignore"
"github.com/sourcegraph/go-diff/diff"
)
Expand Down Expand Up @@ -50,61 +50,65 @@ func CheckDiff(parsedDiff *diff.FileDiff, workspace string) *DiffPaths {
return &diffPaths
}

func ProcessDiffs(raw *diff.Hunk, flagsRef ghc.FlagsRef, flags ldapi.FeatureFlags, aliases map[string][]string, maxFlags int) {
diffRows := strings.Split(string(raw.Body), "\n")
func ProcessDiffs(hunk *diff.Hunk, flagsRef lflags.FlagsRef, flags ldapi.FeatureFlags, aliases map[string][]string, maxFlags int) {
diffRows := strings.Split(string(hunk.Body), "\n")
for _, row := range diffRows {

if (len(flagsRef.FlagsAdded) + len(flagsRef.FlagsRemoved)) >= maxFlags {
if flagsRef.Count() >= maxFlags {
break
}
if strings.HasPrefix(row, "+") {
for _, flag := range flags.Items {
if strings.Contains(row, flag.Key) {
currentKeys := flagsRef.FlagsAdded[flag.Key]
currentKeys = append(currentKeys, "")
flagsRef.FlagsAdded[flag.Key] = currentKeys
}
if len(aliases[flag.Key]) > 0 {
CheckAliasAdded:
for _, alias := range aliases[flag.Key] {
if strings.Contains(row, alias) {
currentKeys := flagsRef.FlagsAdded[flag.Key]
for i := range currentKeys {
if alias == currentKeys[i] {
// If key already exists we do not want to add it
continue CheckAliasAdded
}
}
currentKeys = append(currentKeys, alias)
flagsRef.FlagsAdded[flag.Key] = currentKeys
}
op := operation(row)
for _, flag := range flags.Items {
if strings.Contains(row, flag.Key) {
if op == Add {
if _, ok := flagsRef.FlagsAdded[flag.Key]; !ok {
flagsRef.FlagsAdded[flag.Key] = lflags.AliasSet{}
}
} else if op == Delete {
if _, ok := flagsRef.FlagsRemoved[flag.Key]; !ok {
flagsRef.FlagsRemoved[flag.Key] = lflags.AliasSet{}
}
}
}
} else if strings.HasPrefix(row, "-") {
for _, flag := range flags.Items {
if strings.Contains(row, flag.Key) {
currentKeys := flagsRef.FlagsRemoved[flag.Key]
currentKeys = append(currentKeys, "")
flagsRef.FlagsRemoved[flag.Key] = currentKeys
}
if len(aliases[flag.Key]) > 0 {
CheckAliasRemoved:
for _, alias := range aliases[flag.Key] {
if strings.Contains(row, alias) {
currentKeys := flagsRef.FlagsRemoved[flag.Key]
for i := range currentKeys {
// If key already exists we do not want to add it
if alias == currentKeys[i] {
continue CheckAliasRemoved
}
if len(aliases[flag.Key]) > 0 {
for _, alias := range aliases[flag.Key] {
if strings.Contains(row, alias) {
if op == Add {
if _, ok := flagsRef.FlagsAdded[flag.Key]; !ok {
flagsRef.FlagsAdded[flag.Key] = lflags.AliasSet{}
}
currentKeys = append(currentKeys, alias)
flagsRef.FlagsRemoved[flag.Key] = currentKeys
flagsRef.FlagsAdded[flag.Key][alias] = true
} else if op == Delete {
if _, ok := flagsRef.FlagsRemoved[flag.Key]; !ok {
flagsRef.FlagsRemoved[flag.Key] = lflags.AliasSet{}
}
flagsRef.FlagsRemoved[flag.Key][alias] = true
}
}
}
}
}
}
}

// Operation defines the operation of a diff item.
type Operation int

const (
// Equal item represents an equals diff.
Equal Operation = iota
// Add item represents an insert diff.
Add
// Delete item represents a delete diff.
Delete
)

func operation(row string) Operation {
if strings.HasPrefix(row, "+") {
return Add
}
if strings.HasPrefix(row, "-") {
return Delete
}

return Equal
}
Loading

0 comments on commit 59d4bf1

Please sign in to comment.