Skip to content

Remove dead REST issue-field-value output path#2708

Open
owenniblock wants to merge 1 commit into
mainfrom
owenniblock/remove-dead-rest-field-values
Open

Remove dead REST issue-field-value output path#2708
owenniblock wants to merge 1 commit into
mainfrom
owenniblock/remove-dead-rest-field-values

Conversation

@owenniblock

Copy link
Copy Markdown
Member

Summary

Removes the MinimalIssueFieldValue / MinimalIssueFieldValueSingleSelectOption types and their populator in convertToMinimalIssue. They produce a single_select_option output that is never emittedGetIssue unconditionally cleared minimalIssue.IssueFieldValues before marshalling, and it was the only caller of convertToMinimalIssue.

Why this is safe (dead-code trace)

  • convertToMinimalIssue is the only function that populated MinimalIssue.IssueFieldValues, and its only caller is GetIssue.
  • GetIssue set minimalIssue.IssueFieldValues = nil above the feature-flag check — so no flag, code path, or config could surface it.
  • single_select_option appeared 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_issue to the GraphQL field_values enrichment path — commit message: "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.

Bonus: 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 resurfaced it would have silently dropped multi-select option data.

What changed

  • 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)

MCP impact

  • No tool or API changes — issue_field_values was omitempty and always empty, so output shape is unchanged. The GraphQL field_values path is unaffected. SearchIssueResult.MarshalJSON still suppresses go-github's own embedded issue_field_values.

Lint & tests

  • go build ./..., go vet ./..., go test ./... all pass
  • golangci-lint run — 0 issues

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>
@owenniblock owenniblock requested a review from a team as a code owner June 16, 2026 12:59
Copilot AI review requested due to automatic review settings June 16, 2026 12:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 the MinimalIssue.IssueFieldValues field.
  • Deleted the REST IssueFieldValues population loop in convertToMinimalIssue and the now-redundant clearing in GetIssue.
  • Updated tests to stop asserting on the removed (previously always-empty/omitted) field while preserving GraphQL field_values assertions.
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants