diff --git a/acceptance/main.go b/acceptance/main.go index e499c3eb..7b3a8211 100644 --- a/acceptance/main.go +++ b/acceptance/main.go @@ -35,10 +35,6 @@ func run(ctx context.Context, opts ...githubactions.Option) error { return fmt.Errorf("boilerplate: %w", err) } a := &acceptance{Boilerplate: b} - err = a.syncTodos(ctx) - if err != nil { - return fmt.Errorf("sync todos: %w", err) - } alert, err := a.trigger(ctx) if err != nil { return fmt.Errorf("trigger: %w", err) @@ -163,6 +159,12 @@ func (a *acceptance) notifyIfNeeded(ctx context.Context, alert *notify.Notificat } } if needsIssues { + // doesn't seem to pick the right commit hash, so it's better to do it nightly + // see https://github.com/databrickslabs/ucx/issues/3195 + err := a.syncTodos(ctx) + if err != nil { + return fmt.Errorf("sync todos: %w", err) + } for _, v := range alert.Report { if !v.Failed() { continue diff --git a/acceptance/shim.js b/acceptance/shim.js index 99ae7589..72ed69b6 100644 --- a/acceptance/shim.js +++ b/acceptance/shim.js @@ -1,4 +1,4 @@ -const version = 'v0.4.0'; +const version = 'v0.4.1'; const action = 'acceptance'; const { createWriteStream, chmodSync } = require('fs'); diff --git a/acceptance/todos/finder.go b/acceptance/todos/finder.go index 08cbfb17..635b1ecb 100644 --- a/acceptance/todos/finder.go +++ b/acceptance/todos/finder.go @@ -51,6 +51,7 @@ type TechnicalDebtFinder struct { type Todo struct { message string link string + blame string } func (f *TechnicalDebtFinder) CreateIssues(ctx context.Context) error { @@ -80,7 +81,7 @@ func (f *TechnicalDebtFinder) createIssue(ctx context.Context, todo Todo) error } _, err := f.gh.CreateIssue(ctx, org, repo, github.NewIssue{ Title: fmt.Sprintf("[TODO] %s", todo.message), - Body: fmt.Sprintf("See: %s", todo.link), + Body: fmt.Sprintf("See [more context](%s):\n%s", todo.blame, todo.link), Labels: []string{"tech debt"}, }) if err != nil { @@ -121,6 +122,9 @@ func (f *TechnicalDebtFinder) allTodos(ctx context.Context) ([]Todo, error) { var todos []Todo needle := regexp.MustCompile(`TODO:(.*)`) for _, v := range f.fs { + if !strings.HasSuffix(v.Absolute, v.Relative) { + continue // avoid false positives like https://github.com/databrickslabs/ucx/issues/3193 + } raw, err := v.Raw() if err != nil { return nil, fmt.Errorf("%s: %w", v.Relative, err) @@ -130,9 +134,11 @@ func (f *TechnicalDebtFinder) allTodos(ctx context.Context) ([]Todo, error) { if !needle.MatchString(line) { continue } + link := fmt.Sprintf("%s/%s#L%d-L%d", prefix, v.Relative, i, i+5) todos = append(todos, Todo{ message: strings.TrimSpace(needle.FindStringSubmatch(line)[1]), - link: fmt.Sprintf("%s/%s#L%d-L%d", prefix, v.Relative, i, i+5), + link: link, + blame: strings.ReplaceAll(link, "/blob/", "/blame/"), }) } } @@ -149,6 +155,6 @@ func (f *TechnicalDebtFinder) embedPrefix(ctx context.Context) (string, error) { return "", fmt.Errorf("git history: %w", err) } // example: https://github.com/databrickslabs/ucx/blob/69a0cf8ce3450680dc222150f500d84a1eb523fc/src/databricks/labs/ucx/azure/access.py#L25-L35 - prefix := fmt.Sprintf("https://github.com/%s/%s/blame/%s", org, repo, commits[0].Sha) + prefix := fmt.Sprintf("https://github.com/%s/%s/blob/%s", org, repo, commits[0].Sha) return prefix, nil }