Remove dead REST issue-field-value output path#2708
Open
owenniblock wants to merge 1 commit into
Open
Conversation
The MinimalIssueFieldValue / MinimalIssueFieldValueSingleSelectOption types and their populator in convertToMinimalIssue produced a single_select_option output that is never emitted: GetIssue unconditionally cleared minimalIssue.IssueFieldValues before marshalling, and it was the only caller of convertToMinimalIssue. History: the REST extraction was introduced in #2551 and superseded one day later by #2558, which switched get_issue to the GraphQL field_values enrichment path ("The verbose REST IssueFieldValues is always cleared from the response") to stay consistent with list_issues/search_issues. The struct + populator were left behind as orphaned code. This also removes a latent footgun: the struct only modelled single_select_option (go-github's REST IssueFieldValue has no multi-select equivalent), so any future code that surfaced it would have silently dropped multi-select option data. - Remove MinimalIssueFieldValue + MinimalIssueFieldValueSingleSelectOption - Remove MinimalIssue.IssueFieldValues field (json: issue_field_values) - Remove the populator loop in convertToMinimalIssue - Drop the now-redundant nil assignment in GetIssue - Drop the two test assertions on the removed field (behaviour is now structurally guaranteed; the GraphQL field_values assertions remain) No output shape change: issue_field_values was omitempty and always empty. The GraphQL field_values path is unaffected. SearchIssueResult's MarshalJSON still suppresses go-github's own embedded issue_field_values. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes a dead REST-backed issue_field_values output path from the minimal issue types used by issue_read’s get method. The removed structs and population logic were never observable in tool output because the only caller (GetIssue) always suppressed the field prior to marshalling, and the GraphQL field_values enrichment path remains unchanged.
Changes:
- Removed
MinimalIssueFieldValue*types and theMinimalIssue.IssueFieldValuesfield. - Deleted the REST
IssueFieldValuespopulation loop inconvertToMinimalIssueand the now-redundant clearing inGetIssue. - Updated tests to stop asserting on the removed (previously always-empty/omitted) field while preserving GraphQL
field_valuesassertions.
Show a summary per file
| File | Description |
|---|---|
| pkg/github/minimal_types.go | Deletes unused REST-backed minimal issue-field-value types, removes the issue_field_values field from MinimalIssue, and drops the unused population loop. |
| pkg/github/issues.go | Removes redundant clearing of REST issue field values and keeps GraphQL field_values enrichment behind the feature flag. |
| pkg/github/issues_test.go | Removes assertions tied to the deleted field and keeps coverage for the GraphQL enrichment behavior. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes the
MinimalIssueFieldValue/MinimalIssueFieldValueSingleSelectOptiontypes and their populator inconvertToMinimalIssue. They produce asingle_select_optionoutput that is never emitted —GetIssueunconditionally clearedminimalIssue.IssueFieldValuesbefore marshalling, and it was the only caller ofconvertToMinimalIssue.Why this is safe (dead-code trace)
convertToMinimalIssueis the only function that populatedMinimalIssue.IssueFieldValues, and its only caller isGetIssue.GetIssuesetminimalIssue.IssueFieldValues = nilabove the feature-flag check — so no flag, code path, or config could surface it.single_select_optionappeared in zero test/snapshot expectations; two tests explicitly asserted the field was always empty.History
The REST extraction was added in #2551, then superseded one day later by #2558, which switched
get_issueto the GraphQLfield_valuesenrichment path — commit message: "The verbose REST IssueFieldValues is always cleared from the response" — to stay consistent withlist_issues/search_issues. The struct + populator were left behind as orphaned code.Bonus: removes a latent footgun
The struct only modelled
single_select_option(go-github's RESTIssueFieldValuehas no multi-select equivalent), so any future code that resurfaced it would have silently dropped multi-select option data.What changed
MinimalIssueFieldValue+MinimalIssueFieldValueSingleSelectOptionMinimalIssue.IssueFieldValuesfield (json: issue_field_values)convertToMinimalIssuenilassignment inGetIssuefield_valuesassertions remain)MCP impact
issue_field_valueswasomitemptyand always empty, so output shape is unchanged. The GraphQLfield_valuespath is unaffected.SearchIssueResult.MarshalJSONstill suppresses go-github's own embeddedissue_field_values.Lint & tests
go build ./...,go vet ./...,go test ./...all passgolangci-lint run— 0 issues