Skip to content

Commit

Permalink
Sort issue comments manually. (#69)
Browse files Browse the repository at this point in the history
  • Loading branch information
Gerrit91 authored Apr 3, 2024
1 parent fd5b42f commit aa948df
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 14 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ make swap
# this requires:
# - Ubuntu & Docker
# - KUBECONFIG env var points to the cluster where the robot is deployed
# - the deplyoment in the cluster needs to be called "metal-robot"
# - the deployment in the cluster needs to be called "metal-robot"
#
# when you are done, exit the container shell
# if your session was really long, telepresence sometimes does not swap
Expand Down
2 changes: 1 addition & 1 deletion pkg/clients/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (a *Github) initClients() error {
a.atr = atr
a.itr = itr

a.logger.Info("successfully initalized github app client", "organization-id", a.organizationID, "installation-id", a.installationID, "expected-events", installation.Events)
a.logger.Info("successfully initialized github app client", "organization-id", a.organizationID, "installation-id", a.installationID, "expected-events", installation.Events)

return nil
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/config/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type DocsPreviewCommentConfig struct {
}

type AggregateReleasesConfig struct {
TargetRepositoryName string `mapstructure:"repository" description:"the name of the taget repo"`
TargetRepositoryName string `mapstructure:"repository" description:"the name of the target repo"`
TargetRepositoryURL string `mapstructure:"repository-url" description:"the url of the target repo"`
Branch *string `mapstructure:"branch" description:"the branch to push in the target repo"`
BranchBase *string `mapstructure:"branch-base" description:"the base branch to raise the pull request against"`
Expand All @@ -44,7 +44,7 @@ type DistributeReleasesConfig struct {
}

type YAMLTranslateReleasesConfig struct {
TargetRepositoryName string `mapstructure:"repository" description:"the name of the taget repo"`
TargetRepositoryName string `mapstructure:"repository" description:"the name of the target repo"`
TargetRepositoryURL string `mapstructure:"repository-url" description:"the url of the target repo"`
Branch *string `mapstructure:"branch" description:"the branch to push in the target repo"`
BranchBase *string `mapstructure:"branch-base" description:"the base branch to raise the pull request against"`
Expand All @@ -55,7 +55,7 @@ type YAMLTranslateReleasesConfig struct {

type YAMLTranslation struct {
From YAMLTranslationRead `mapstructure:"from" description:"the yaml path from where to read the replacement value"`
To []Modifier `mapstructure:"to" description:"the actions to take on the traget repo with the read the replacement value"`
To []Modifier `mapstructure:"to" description:"the actions to take on the target repo with the read the replacement value"`
}

type YAMLTranslationRead struct {
Expand Down
13 changes: 12 additions & 1 deletion pkg/webhooks/github/actions/aggregate_releases.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"log/slog"
"net/url"
"sort"
"strings"
"sync"

Expand Down Expand Up @@ -51,7 +52,7 @@ func NewAggregateReleases(logger *slog.Logger, client *clients.Github, rawConfig
)

var typedConfig config.AggregateReleasesConfig
err := mapstructure.Decode(rawConfig, &typedConfig)
err := mapstructure.Decode(rawConfig, &typedConfig) // nolint:musttag
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -246,6 +247,10 @@ func isReleaseFreeze(ctx context.Context, client *v3.Client, number int, owner,
return true, fmt.Errorf("unable to list pull request comments: %w", err)
}

// somehow the direction parameter has no effect, it's always sorted in the same way?
// therefore sorting manually:
sort.Slice(comments, sortComments(comments))

for _, comment := range comments {
comment := comment

Expand All @@ -260,3 +265,9 @@ func isReleaseFreeze(ctx context.Context, client *v3.Client, number int, owner,

return false, nil
}

func sortComments(comments []*v3.IssueComment) func(i, j int) bool {
return func(i, j int) bool {
return comments[j].CreatedAt.Before(comments[i].CreatedAt.Time)
}
}
53 changes: 53 additions & 0 deletions pkg/webhooks/github/actions/aggregate_releases_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package actions

import (
"sort"
"testing"
"time"

"github.com/google/go-cmp/cmp"
v3 "github.com/google/go-github/v57/github"
"github.com/metal-stack/metal-lib/pkg/pointer"
)

func Test_sortComments(t *testing.T) {
now := time.Now()

tests := []struct {
name string
comments []*v3.IssueComment
want []*v3.IssueComment
}{
{
name: "newest comment should appear first in the list",
comments: []*v3.IssueComment{
{
ID: pointer.Pointer(int64(1)),
CreatedAt: &v3.Timestamp{Time: now.Add(-3 * time.Minute)},
},
{
ID: pointer.Pointer(int64(2)),
CreatedAt: &v3.Timestamp{Time: now.Add(2 * time.Minute)},
},
},
want: []*v3.IssueComment{
{
ID: pointer.Pointer(int64(2)),
CreatedAt: &v3.Timestamp{Time: now.Add(2 * time.Minute)},
},
{
ID: pointer.Pointer(int64(1)),
CreatedAt: &v3.Timestamp{Time: now.Add(-3 * time.Minute)},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
sort.Slice(tt.comments, sortComments(tt.comments))
if diff := cmp.Diff(tt.comments, tt.want); diff != "" {
t.Errorf("diff: %s", diff)
}
})
}
}
2 changes: 1 addition & 1 deletion pkg/webhooks/github/actions/distribute_releases.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func newDistributeReleases(logger *slog.Logger, client *clients.Github, rawConfi
)

var typedConfig config.DistributeReleasesConfig
err := mapstructure.Decode(rawConfig, &typedConfig)
err := mapstructure.Decode(rawConfig, &typedConfig) // nolint:musttag
if err != nil {
return nil, err
}
Expand Down
10 changes: 4 additions & 6 deletions pkg/webhooks/github/actions/release_drafter.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
"github.com/metal-stack/metal-robot/pkg/markdown"
"github.com/metal-stack/metal-robot/pkg/utils"
"github.com/mitchellh/mapstructure"

v3 "github.com/google/go-github/v57/github"
)

var (
Expand Down Expand Up @@ -393,7 +391,7 @@ func ensureReleaseSection(m *markdown.Markdown, headline string) *markdown.Markd
}

type releaseInfos struct {
existing *v3.RepositoryRelease
existing *github.RepositoryRelease
releaseTag string
body string
}
Expand Down Expand Up @@ -481,10 +479,10 @@ func (r *releaseDrafter) createOrUpdateRelease(ctx context.Context, infos *relea
r.logger.Info("release draft updated", "repository", r.repoName, "trigger-component", p.RepositoryName, "version", p.TagName)
} else {
newDraft := &github.RepositoryRelease{
TagName: v3.String(infos.releaseTag),
Name: v3.String(fmt.Sprintf(r.titleTemplate, infos.releaseTag)),
TagName: github.String(infos.releaseTag),
Name: github.String(fmt.Sprintf(r.titleTemplate, infos.releaseTag)),
Body: &body,
Draft: v3.Bool(true),
Draft: github.Bool(true),
}
_, _, err := r.client.GetV3Client().Repositories.CreateRelease(ctx, r.client.Organization(), r.repoName, newDraft)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/webhooks/github/actions/yaml_translate_releases.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func newYAMLTranslateReleases(logger *slog.Logger, client *clients.Github, rawCo
)

var typedConfig config.YAMLTranslateReleasesConfig
err := mapstructure.Decode(rawConfig, &typedConfig)
err := mapstructure.Decode(rawConfig, &typedConfig) // nolint:musttag
if err != nil {
return nil, err
}
Expand Down

0 comments on commit aa948df

Please sign in to comment.