Remove TODOs

model/build/db.go

@@ -277,7 +277,6 @@ func SetBuildStartedForTasks(tasks []task.Task, caller string) error {
 
 // FindByVersionAndVariants finds all builds that are in the given version and
 // match one of the build variant names.
-// kim: TODO: add small unit test
 func FindByVersionAndVariants(ctx context.Context, version string, variants []string) ([]Build, error) {
 	if len(variants) == 0 {
 		return nil, nil

model/generate.go

@@ -374,12 +374,7 @@ func (g *GeneratedProject) saveNewBuildsAndTasks(ctx context.Context, settings *
 // that have added generated tasks to run. For example, if a build was already
 // finished and just had new tasks generated, the status should be updated to
 // running.
-// kim: TODO: add Save tests for build status updates.
 func updateBuildStatusesForGeneratedTasks(ctx context.Context, versionID string, newTVPairsForExistingVariants TaskVariantPairs) error {
-	// kim: NOTE: first need to get build IDs of existing builds that had tasks
-	// added to determine which builds may need their status updated. The
-	// task/build creation functions don't return the created tasks/builds, so
-	// we have to query them.
 	bvs := getBuildVariantsFromPairs(newTVPairsForExistingVariants)
 
 	builds, err := build.FindByVersionAndVariants(ctx, versionID, bvs)
@@ -392,10 +387,6 @@ func updateBuildStatusesForGeneratedTasks(ctx context.Context, versionID string,
 		buildIDs = append(buildIDs, b.Id)
 	}
 
-	// kim: NOTE: I didn't verify, but it may be sufficient to call
-	// UpdateVersionAndPatchStatusForBuilds here to ensure the build statuses
-	// are updated for new tasks. Should write unit test and verify that it
-	// fails before change and succeeds after change.
 	return UpdateVersionAndPatchStatusForBuilds(ctx, buildIDs)
 }
 

model/task_lifecycle.go

@@ -1600,19 +1600,6 @@ func UpdatePatchStatus(ctx context.Context, p *patch.Patch, status string) error
 // UpdateBuildAndVersionStatusForTask updates the status of the task's build based on all the tasks in the build
 // and the task's version based on all the builds in the version.
 // Also update build and version Github statuses based on the subset of tasks and builds included in github checks
-// kim: NOTE: this runs at the end of a task to update its build, then update
-// the version assuming just this task's build changed.
-// kim: NOTE: it uses the cached build statuses to compute the overall version
-// status. It does not re-evaluate every single build. If generate.tasks creates
-// new tasks in a build, I'm pretty sure it doesn't update the build/version
-// status to reflect that. That means this could happen:
-// * generate_resmoke_build_variants runs and creates new builds/tasks, build is
-// "created"
-// * all the tasks in the new build finish, build is "success"
-// * another generator runs and adds new tasks to existing builds, build is
-// "success" (because it wasn't updated by the generator)
-// * generate.tasks finishes and reports the version status as success because
-// all the version's builds are finished.
 func UpdateBuildAndVersionStatusForTask(ctx context.Context, t *task.Task) error {
 	ctx, span := tracer.Start(ctx, "save-builds-and-tasks")
 	defer span.End()