`clear-pr-merge-commit-message` - Limit to squash merges by Copilot · Pull Request #9398 · refined-github/refined-github · GitHub
Skip to content

clear-pr-merge-commit-message - Limit to squash merges#9398

Merged
fregante merged 6 commits into
mainfrom
copilot/fix-clear-pr-merge-commit-message
May 9, 2026
Merged

clear-pr-merge-commit-message - Limit to squash merges#9398
fregante merged 6 commits into
mainfrom
copilot/fix-clear-pr-merge-commit-message

Conversation

Copilot AI commented May 8, 2026 •
edited by fregante
Loading

Copy link
Copy Markdown
Contributor

Copilot AI linked an issue May 8, 2026 that may be closed by this pull request
clear-pr-merge-commit-message should not clear non-squashed merge commit messages #8399
Closed
Copilot AI and others added 3 commits May 8, 2026 17:36
Copilot AI changed the title [WIP] Fix clear PR merge commit message behavior clear-pr-merge-commit-message - Only clear squash merge commit messages May 8, 2026
Copilot AI requested a review from fregante May 8, 2026 17:40
fregante
fregante requested changes May 8, 2026
View reviewed changes
Comment thread source/features/clear-pr-merge-commit-message.tsx Outdated
Comment thread source/features/clear-pr-merge-commit-message.tsx Outdated
Comment thread source/features/clear-pr-merge-commit-message.tsx Outdated
Copilot AI requested a review from fregante May 8, 2026 18:03
@SunsetTechuila

SunsetTechuila commented May 8, 2026

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot added the bug label May 9, 2026
@fregante fregante changed the title clear-pr-merge-commit-message - Only clear squash merge commit messages clear-pr-merge-commit-message - Limit to squash merges May 9, 2026
@fregante

fregante commented May 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

@SunsetTechuila looks like it can be simplified in that feature since /squash/i works.

And actually, no-optional-chaining strikes again. needsSubmission won't be called unless the confirm button exists, so all those nullish checks should be dropped.

I'll open a followup PR because it looks like most optional usage there is wrong

fregante
fregante approved these changes May 9, 2026
View reviewed changes

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

Tested ✅

@fregante fregante marked this pull request as ready for review May 9, 2026 09:03
@SunsetTechuila

SunsetTechuila commented May 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

And actually, no-optional-chaining strikes again. needsSubmission won't be called unless the confirm button exists, so all those nullish checks should be dropped.

Wrong. needsSubmission can be called (by updateCommitTitle) when the PR title is updated, which can happen when the merge box isn't in the confirm state. Please be very careful when updating the code.

@SunsetTechuila SunsetTechuila mentioned this pull request May 9, 2026
Merged
@fregante fregante merged commit c5b4ea2 into main May 9, 2026
21 checks passed
@fregante

fregante commented May 9, 2026

Copy link
Copy Markdown
Member

Wrong.

So you're saying that code was not self-evident and a contributor didn't know when it could be optional, leading to a bad refactor? Sounds like what I said when introducing mandatory "when" comments :)

@fregante fregante deleted the copilot/fix-clear-pr-merge-commit-message branch May 9, 2026 09:40
@SunsetTechuila

SunsetTechuila commented May 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Reviewers

1 more reviewer
@fregante fregante fregante approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@fregante fregante

Copilot code review Copilot

Labels

bug

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

clear-pr-merge-commit-message should not clear non-squashed merge commit messages

3 participants

@SunsetTechuila @fregante