Skip to content

Fix 422 error when updating release without commitish parameter#144

Merged
taylorsilva merged 2 commits intoconcourse:masterfrom
bonzofenix:fix-commitish-422-error
Nov 24, 2025
Merged

Fix 422 error when updating release without commitish parameter#144
taylorsilva merged 2 commits intoconcourse:masterfrom
bonzofenix:fix-commitish-422-error

Conversation

@bonzofenix
Copy link
Contributor

When updating an existing release without providing a commitish parameter, the resource was setting TargetCommitish to an empty string, which GitHub's API rejects with a 422 "Invalid target_commitish parameter" error.

This fix mirrors the behavior of the Body field - when commitish is not specified, TargetCommitish is now set to nil instead of an empty string, allowing GitHub to preserve the existing target commit for the release.

@bonzofenix bonzofenix requested a review from a team as a code owner October 27, 2025 18:32
@bonzofenix bonzofenix force-pushed the fix-commitish-422-error branch from 2b9bcc0 to b9217d2 Compare November 7, 2025 13:17
When updating an existing release without providing a commitish parameter,
the resource was setting TargetCommitish to an empty string, which GitHub's
API rejects with a 422 "Invalid target_commitish parameter" error.

This fix mirrors the behavior of the Body field - when commitish is not
specified, TargetCommitish is now set to nil instead of an empty string,
allowing GitHub to preserve the existing target commit for the release.

Signed-off-by: Alan Moran <bonzofenix@gmail.com>
@bonzofenix bonzofenix force-pushed the fix-commitish-422-error branch from b9217d2 to 6c32a1a Compare November 7, 2025 13:21
out_command.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

targetCommitish != ""?

Copy link
Member

@taylorsilva taylorsilva Nov 24, 2025

Choose a reason for hiding this comment

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

Using more words:

Instead of introducing a new variable, why not check if targetCommitish != ""?

Copy link
Member

@taylorsilva taylorsilva left a comment

Choose a reason for hiding this comment

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

Mostly there, just some nit-picks.

out_command.go Outdated
Copy link
Member

@taylorsilva taylorsilva Nov 24, 2025

Choose a reason for hiding this comment

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

Using more words:

Instead of introducing a new variable, why not check if targetCommitish != ""?

out_command.go Outdated
Comment on lines 104 to 106
Copy link
Member

Choose a reason for hiding this comment

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

The else block is redundant because existingRelease.TargetCommitish will be nil by default.

Signed-off-by: Taylor Silva <dev@taydev.net>
@taylorsilva
Copy link
Member

I've pushed a commit to your branch which makes the nit-pick changes I suggested. Thanks again for the PR and making this resource more compliant with the Github API. I'll merge once the unit tests in CI are done running.

@taylorsilva taylorsilva merged commit f83a19e into concourse:master Nov 24, 2025
2 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Pull Requests Nov 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants

Comments