x/build/cmd/releasebot: in uploadStagingRelease method, target parameter is unused

[dmitshur]

Consider the current code of the Work.uploadStagingRelease method in package golang.org/x/build/cmd/releasebot:

// uploadStagingRelease uploads target to the release staging bucket.
// If successful, it records the corresponding URL in out.Link.
// In addition to uploading target, it creates and uploads a file
// named "<target>.sha256" containing the hex sha256 hash
// of the target file. This is needed for the release signing process
// and also displayed on the eventual download page.
func (w *Work) uploadStagingRelease(target string, out *ReleaseOutput) error {
	src := filepath.Join(w.runDir, out.File)
	h := sha256.New()
	f, err := os.Open(src)
	if err != nil {
		return err
	}
	_, err = io.Copy(h, f)
	f.Close()
	if err != nil {
		return err
	}
	if err := ioutil.WriteFile(src+".sha256", []byte(fmt.Sprintf("%x", h.Sum(nil))), 0666); err != nil {
		return err
	}

	dst := w.Version + "/" + out.File
	if err := gcsUpload(src, dst); err != nil {
		return err
	}
	if err := gcsUpload(src+".sha256", dst+".sha256"); err != nil {
		return err
	}

	w.releaseMu.Lock()
	out.Link = "https://" + releaseBucket + ".storage.googleapis.com/" + dst
	w.releaseMu.Unlock()
	return nil
}

What did you expect to see?

The target string parameter should either be used, or it should be removed from the method signature (and not mentioned in the comment).

What did you see instead?

Documentation talks about target (repeatedly), but the target parameter is unused in the function body.

/cc @rsc You wrote this code in CL 73333.

[FiloSottile]

/cc @andybons

[andybons]

NeedsDecision will trigger review at the weekly proposal meeting. Marking as NeedsInvestigation.

[dmitshur]

After working with this code in CL 234531, I'm pretty sure I understand what happened.

In the documentation of the uploadStagingRelease method, the word "target" really means the "release target". The out *ReleaseOutput parameter provides all the information needed for the upload operation.

It's likely the target string parameter was added in case the target name needed to be logged, but it just happened not to be.

So, this is harmless, and will be fixed by CL 234531.

[gopherbot]

Change https://golang.org/cl/234531 mentions this issue: cmd/releasebot, cmd/release: include long tests in release process