Skip to content

refactor: remove redundant merge information text from pull request view#235

Open
pieer wants to merge 3 commits into
masterfrom
fix/remove-branch-deletion-message-after-merge
Open

refactor: remove redundant merge information text from pull request view#235
pieer wants to merge 3 commits into
masterfrom
fix/remove-branch-deletion-message-after-merge

Conversation

@pieer

@pieer pieer commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Issue: The custom template unconditionally renders "repo.pulls.merged_info_text", which in the custom locale says "The branch %s can now be deleted." — shown after every merge, with no condition on whether the branch is actually deletable.

Here's exactly what's still broken:
custom/templates/repo/issue/view_content/pull_merge_box.tmpl:48-50:

  <div class="merge-section-info">
      {{ctx.Locale.Tr "repo.pulls.merged_info_text" (HTMLFormat "<code>%s</code>" .HeadTarget)}}
  </div>

This renders unconditionally after every merge. Since branches are always auto-deleted on this project, it's always wrong.

The fix is simple — remove those 3 lines from the template (the locale key can stay, it's harmless). The comment on issue #187 that said "fixed in #166" was premature; only half the issue was addressed.

Co-author with Claude opus 4.8

@pieer pieer self-assigned this Jun 29, 2026
@pieer

pieer commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

Summary

Removes the three-line merge-section-info block from the custom pull_merge_box.tmpl that unconditionally rendered "The branch %s can now be deleted." after every merge. Forkana auto-deletes branches and has already stripped its branch-deletion UI (#182), so the message was always misleading and had no accompanying action. The change is minimal, correct, and leaves the surrounding {{if}}/{{else if}} structure intact; the findings below are all low-importance cleanups/observations rather than defects in the change itself.

Findings

1. 🟡 Removal hardcodes the "branches are always auto-deleted" assumption

File: custom/templates/repo/issue/view_content/pull_merge_box.tmpl:42-49
Problem: The upstream template wraps this same text in {{if .IsPullBranchDeletable}} and pairs it with a delete-branch button; Forkana's override dropped the guard (the real bug) and this PR deletes the text outright. Auto-delete is a per-repo default (PullRequestsConfig.DefaultDeleteBranchAfterMerge) that a user can untick at merge time, so a head branch can still survive a merge — in that case Forkana now shows neither a hint nor any way to delete it. This is acceptable given branch-deletion UI was intentionally removed in #182 (a message with no actionable button would be useless), but the assumption is worth recording: if Forkana does not actually enforce auto-delete, leftover branches become invisible in the PR view.

{{/* If Forkana ever wants to surface leftover branches again, restore the upstream
     guard AND a delete affordance together, rather than the message alone: */}}
{{if .IsPullBranchDeletable}}
  ...heading + merge-section-info + delete button...
{{end}}

Status: open

  • Addressed
  • Dismissed

2. ⚪️ The custom locale override pulls.merged_info_text is now dead

File: custom/options/locale/locale_en-US.ini:1999
Problem: After this change the custom template no longer references repo.pulls.merged_info_text, and since the custom pull_merge_box.tmpl fully shadows the base template, the key is no longer rendered anywhere in Forkana; the custom override is now dead weight. The PR body deliberately keeps it ("harmless"), which is fine, but removing the custom override line would prevent confusion about whether it is still wired up.

; custom/options/locale/locale_en-US.ini — safe to drop this now-unused override
; pulls.merged_info_text = The branch %s can now be deleted.

Status: open

  • Addressed
  • Dismissed

3. ⚪️ tw-mb-2 on the now-last child leaves trailing whitespace under the heading

File: custom/templates/repo/issue/view_content/pull_merge_box.tmpl:44-46
Problem: With the info <div> gone, the <h3 class="tw-mb-2">{{Tr "repo.pulls.merged_success"}}</h3> is the only child of item-section-left, so its bottom margin now adds dangling space below "Successfully merged" with nothing beneath it. Minor cosmetic; drop tw-mb-2 (or verify the spacing still looks intentional).

<h3>
	{{ctx.Locale.Tr "repo.pulls.merged_success"}}
</h3>

Status: open

  • Addressed
  • Dismissed

Counts

Severity Count
🔴 High 0
🟡 Medium 1
⚪️ Low 2

pieer and others added 2 commits June 30, 2026 15:11
The custom pull_merge_box template no longer references
repo.pulls.merged_info_text (and it fully shadows the base template), so this
custom locale override is dead. Remove it to avoid implying it's still wired up.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
With the merge-info line removed, the "merged_success" <h3> is the only child
of item-section-left, so its tw-mb-2 left dangling whitespace beneath it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant