chore: Factor out sub-query for locating nearest uploads#64210
Conversation
kritzcreek
left a comment
There was a problem hiding this comment.
One comment update and a small nit
| repositoryID, dbutil.CommitBytea(commit), | ||
| repositoryID, dbutil.CommitBytea(commit), | ||
| ) | ||
| // visible from the given set of commits. This is done by removing the |
There was a problem hiding this comment.
You say "from the given set of commits" here, but then further down "This function deliberately accepts a single commit". Which is it?
There was a problem hiding this comment.
Good catch, this function was changed to accept a list in the middle of refactoring and then I changed it back once I realized the problem with correctness.
| func makeNearestUploadsQuery(repositoryID api.RepoID, commits ...api.CommitID) *sqlf.Query { | ||
| commitQueries := sqlf.Join(genslices.Map(commits, func(commit api.CommitID) *sqlf.Query { | ||
| return sqlf.Sprintf("%s", dbutil.CommitBytea(commit)) | ||
| }), ", ") |
There was a problem hiding this comment.
nit: I've found that because of the lack of inference for lambdas, it's sometimes a little easier to read genslice code when you pull out a small top-level function.
| func makeNearestUploadsQuery(repositoryID api.RepoID, commits ...api.CommitID) *sqlf.Query { | |
| commitQueries := sqlf.Join(genslices.Map(commits, func(commit api.CommitID) *sqlf.Query { | |
| return sqlf.Sprintf("%s", dbutil.CommitBytea(commit)) | |
| }), ", ") | |
| func formatCommit(commit api.CommitID) *sqlf.Query { | |
| return sqlf.Sprintf("%s", dbutil.CommitBytea(commit)) | |
| } | |
| func makeNearestUploadsQuery(repositoryID api.RepoID, commits ...api.CommitID) *sqlf.Query { | |
| commitQueries := sqlf.Join(genslices.Map(commits, formatCommit), ", ") |
There was a problem hiding this comment.
I've changed this to use a local function instead of a top-level function since sometimes commits are used as text columns, so the name might be misleading if it were a top-level function.
A large sub-query was duplicated between two queries, making
the code harder to understand. This patch factors that out.
Ancillary changes:
Test plan
Covered by existing tests