Skip to content

Trending and tests#17

Merged
raymondjacobson merged 4 commits intomainfrom
rj-trending-queries
Apr 11, 2025
Merged

Trending and tests#17
raymondjacobson merged 4 commits intomainfrom
rj-trending-queries

Conversation

@raymondjacobson
Copy link
Copy Markdown
Member

very simple, still some diffs to consider
image

Copy link
Copy Markdown
Contributor

@stereosteve stereosteve left a comment

Choose a reason for hiding this comment

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

Looks nice. Couple of notes.

Comment thread api/v1_trending.go
sql := `
SELECT track_trending_scores.track_id
FROM track_trending_scores
LEFT JOIN tracks
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't this be a full join?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i dont think so. if there's no scores for a track, we probably don't care about it

Comment thread api/v1_trending.go Outdated
Comment on lines +43 to +55
type trackTrendingRow struct {
TrackId int32 `json:"track_id"`
}

trackTrendingRows, err := pgx.CollectRows(rows, pgx.RowToStructByName[trackTrendingRow])
if err != nil {
return err
}

trackIds := []int32{}
for _, t := range trackTrendingRows {
trackIds = append(trackIds, t.TrackId)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if just selecting one column scalar value, you can simplify this to:

trackIds, err := pgx.CollectRows(rows, pgx.RowTo[int32])
if err != nil {
  return err
}

see:
https://pkg.go.dev/github.com/jackc/pgx/v5#hdr-Query_Interface

Comment thread api/fixture_test.go Outdated
"created_at": time.Now(),
}

connectedWalletsBaseRow = map[string]any{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this indentation looks off... you have gofmt setup to run on save?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed, bad rebase thing

@raymondjacobson raymondjacobson merged commit 7abf608 into main Apr 11, 2025
2 checks passed
@raymondjacobson raymondjacobson deleted the rj-trending-queries branch April 11, 2025 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants