Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clear-pr-merge-commit-message preserve closing issues on non-default branch #6122

Merged
merged 12 commits into from Nov 1, 2022

Conversation

yakov116
Copy link
Member

@yakov116 yakov116 commented Oct 31, 2022

Fixes #4531

Test URLs

refined-github/sandbox#53

Screenshot

image

source/features/clear-pr-merge-commit-message.tsx Outdated Show resolved Hide resolved
source/features/clear-pr-merge-commit-message.tsx Outdated Show resolved Hide resolved
source/features/clear-pr-merge-commit-message.tsx Outdated Show resolved Hide resolved
source/features/clear-pr-merge-commit-message.tsx Outdated Show resolved Hide resolved

const issueNumberElement = keyword.nextElementSibling as HTMLAnchorElement;
const isCrossRepo = getRepo(issueNumberElement)!.nameWithOwner !== getRepo()!.nameWithOwner;
const issueNumber = isCrossRepo ? issueNumberElement.href : issueNumberElement.textContent!;
Copy link
Member

Choose a reason for hiding this comment

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

What if we just get the whole URL? fixes https://GitHub.com/user/repo/issues/123

It works regardless of the repo. I'm not particularly worried about this scenario to be honest because the users who want this specific behavior can just disable the feature and user GitHub's own repo config.

Fixing it is ok, but having a clean fixes #124 isn't worth the complexity here.

@yakov116 yakov116 merged commit 43ef4e5 into main Nov 1, 2022
@yakov116 yakov116 deleted the fix_closing branch November 1, 2022 13:26
fregante added a commit to refined-github/sandbox that referenced this pull request Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 participants