Skip to content

fix(KONFLUX-141173): remove footnotes on git reporters#1586

Merged
NigelByrne1 merged 1 commit into
konflux-ci:mainfrom
NigelByrne1:konflux-14173
Jun 16, 2026
Merged

fix(KONFLUX-141173): remove footnotes on git reporters#1586
NigelByrne1 merged 1 commit into
konflux-ci:mainfrom
NigelByrne1:konflux-14173

Conversation

@NigelByrne1

Copy link
Copy Markdown
Contributor
  • notes column now displays TEST_OUTPUT.note
  • footnotes are now duplicating this data
  • footnotes are misleading in some git reporters
  • removing them as they are no longer required

assisted by ai: Cursor

Maintainers will complete the following section

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request removes the footnote formatting functionality from the test summary output, simplifying the generated markdown and updating the corresponding tests. The review feedback suggests further refactoring FormatTaskName to remove an obsolete call to GetTestResult(), which will improve performance and simplify the code.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread status/format.go Outdated
@codecov-commenter

codecov-commenter commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.09%. Comparing base (eb20c77) to head (6bc044b).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1586      +/-   ##
==========================================
- Coverage   72.86%   72.09%   -0.77%     
==========================================
  Files          63       63              
  Lines        8413     8809     +396     
==========================================
+ Hits         6130     6351     +221     
- Misses       1658     1817     +159     
- Partials      625      641      +16     
Flag Coverage Δ
e2e-tests 49.17% <100.00%> (+0.04%) ⬆️
unit-tests 65.36% <100.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 4, 2026

Copy link
Copy Markdown

Review

Findings

No findings.

The PR cleanly removes the footnote formatting feature from git reporters. The FormatFootnotes function, its template invocation, and the footnote-reference logic in FormatTaskName are all removed together with no stale references remaining. Test expectations are updated consistently — footnote references removed from task names, footnote definitions removed from expected output, and the extra blank lines previously produced by the empty formatFootnotes output are correctly cleaned up. The FormatNote function remains to display notes in the table column, confirming the PR description's claim that footnotes were duplicating data already shown there.

Previous run

Review

Findings

Low

  • [behavioral-change] status/format.go:211FormatTaskName no longer appends [^name] footnote reference suffix when a note exists. The function signature is unchanged but return values differ for tasks with notes. Verified safe: no external consumers of this function exist; it is only used as a template function within FormatTestsSummary. Tests are updated to match.

Info

  • [output-format-change] status/format.go:282 — Exported function FormatFootnotes is removed. Analysis confirms it was only used internally as a template function. No external repositories depend on the status package.
  • [scope-tier-alignment] — The PR uses fix() commit type for what is closer to a feature removal/simplification. Removing footnotes that are "misleading in some git reporters" is a reasonable interpretation of a fix, but could also be classified as refactor(). Minor observation, no action needed.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Jun 4, 2026
 - notes column now displays TEST_OUTPUT.note
 - footnotes are now duplicating this data
 - footnotes are misleading in some git reporters
 - removing them as they are no longer required

assisted by ai: Cursor

Signed-off-by: nbyrne <nbyrne@redhat.com>
@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels Jun 8, 2026
@NigelByrne1

Copy link
Copy Markdown
Contributor Author

/retest

1 similar comment
@NigelByrne1

Copy link
Copy Markdown
Contributor Author

/retest

@dirgim dirgim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@jsztuka jsztuka 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.

lgtm

@jsztuka

jsztuka commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

/retest

@jsztuka

jsztuka commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

/retest

1 similar comment
@NigelByrne1

Copy link
Copy Markdown
Contributor Author

/retest

@kasemAlem

Copy link
Copy Markdown
Contributor

failed to provision cluster
/retest

@NigelByrne1

Copy link
Copy Markdown
Contributor Author

/retest

@red-hat-konflux

Copy link
Copy Markdown
Contributor

All PipelineRuns for this commit have already succeeded. Use /retest <pipeline-name> to re-run a specific pipeline or /test to re-run all pipelines.

@jsztuka

jsztuka commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

/retest

@red-hat-konflux

Copy link
Copy Markdown
Contributor

All PipelineRuns for this commit have already succeeded. Use /retest <pipeline-name> to re-run a specific pipeline or /test to re-run all pipelines.

@NigelByrne1

Copy link
Copy Markdown
Contributor Author

/test

2 similar comments
@NigelByrne1

Copy link
Copy Markdown
Contributor Author

/test

@jsztuka

jsztuka commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

/test

@kasemAlem

Copy link
Copy Markdown
Contributor

/retest

1 similar comment
@NigelByrne1

Copy link
Copy Markdown
Contributor Author

/retest

@NigelByrne1 NigelByrne1 merged commit 1d77a40 into konflux-ci:main Jun 16, 2026
26 checks passed
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jun 16, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 12:41 PM UTC · Completed 12:49 PM UTC
Commit: ffde3b2 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #1586 — Remove footnotes on git reporters

Workflow quality: Good. This was a clean human-authored PR (4 additions, 47 deletions, 3 files) that the review agent handled well overall. Two review runs, four human approvals, no rework cycles. Time to merge (12 days) was due to CI cluster provisioning failures requiring multiple /retest cycles — not an agent issue.

One gap identified: The fullsend review agent approved the PR but missed an optimization that gemini-code-assist caught: after removing footnote logic, FormatTaskName always returns name, nil regardless of test result, making the GetTestResult() call (JSON unmarshaling + schema validation) unnecessary dead code. This is a "refactoring completeness" check the review agent could perform.

Skipped proposals (covered by existing issues):

Proposals filed

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

Labels

ready-for-merge All reviewers approved — ready to merge size: S size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants