Conversation
Signed-off-by: Eric Pickard <piceri@github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the deployment record creation request to set return_records=false, reducing response payload/bandwidth since the deployment tracker doesn’t use the response body from the create call.
Changes:
- Replace direct
json.Marshal(record)with a helper that injectsreturn_records=falseinto the request body. - Add/extend a request-validation test to assert the new request field is present.
Show a summary per file
| File | Description |
|---|---|
| pkg/deploymentrecord/client.go | Build request JSON with return_records=false when posting a deployment record. |
| pkg/deploymentrecord/client_test.go | Validate the POST body includes return_records:false. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 2
| } | ||
| body, err := io.ReadAll(r.Body) | ||
| if err != nil { | ||
| t.Fatal("unable to read request body") |
There was a problem hiding this comment.
The test failure message drops the underlying read error, which makes debugging intermittent test/server issues harder. Include err in the fatal message (or use t.Fatalf) so the failure has actionable context.
| t.Fatal("unable to read request body") | |
| t.Fatalf("unable to read request body: %v", err) |
| if !bytes.Contains(body, []byte("\"return_records\":false")) { | ||
| t.Error("expected '\"return_records\":false' in the request body") | ||
| } |
There was a problem hiding this comment.
Using a raw substring check on the request body can yield false positives (it doesn't validate JSON structure/typing). Consider unmarshalling the body and asserting return_records is a boolean false to make this test more robust.
malancas
left a comment
malancas
left a comment
There was a problem hiding this comment.
The Copilot suggestions also make sense to me
This changes sets return_records to false when creating deployment records. This will reduce bandwidth as deployment tracker does not use the response body content from creating deployment records.
https://docs.github.com/en/rest/orgs/artifact-metadata?apiVersion=2026-03-10#create-an-artifact-deployment-record