Skip to content

Automatically fixup linked issues in subtree repository #1897

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

Merged
merged 3 commits into from
Apr 7, 2025

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Feb 4, 2025

This PR adds a new handler to "relink" linked GitHub issues into their unambiguous form so that when pulling commits from a subtree into the main repository we don't accidentally closes issues in the wrong repository.

Example: Fixes #123 (in rust-lang/clippy) would be automatically changed to Fixes rust-lang/clippy#123

r? @ehuss

@Urgau Urgau force-pushed the relink-handler branch 2 times, most recently from 21ac445 to 54f8126 Compare February 4, 2025 21:20
@jackh726
Copy link
Member

I think we should gate this behind a manual command to do the relinking, in case there are issues we don't wreck everyone's PR descriptions.

@Urgau
Copy link
Member Author

Urgau commented Feb 14, 2025

It's already gated behind the [relink] config, if there are any issue (which is very unlikely IMO) the relink can be disabled by removing the section from the configuration file.

Putting the relink behind a manual command renders (IMO) this handler/pass useless, as I don't think anyone is going to use it and the situation is going to stay the same without any improvements.

If we are worried about edge-cases we might miss, we can do a slow deployment, starting with lower traffic repos, and go higher trafic repo as we have more confidence in it.

@jackh726
Copy link
Member

Hmm, I suppose it's fine to land this without a gate if we start with a slow repo and allow plenty of testing time

@ehuss
Copy link
Contributor

ehuss commented Mar 26, 2025

This doesn't seem like it will help if the link is in a commit description, correct? If so, I'm not sure if it will help much if the common case is someone entering it in a commit.

@Urgau
Copy link
Member Author

Urgau commented Mar 26, 2025

No indeed, if the "fixes" is in the commit it-self this handler won't help. Maybe we should give a warning when we cannot just fix it ourselves.

Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Left one question, otherwise the implementation looks good to me, and I think that if we opt into it on some subtree repo, it should be fine.

Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

I tested this on my test repo and it seems to work fine (modulo one nit).

Since this is gated behind an option, I'm fine with landing it after the nit is fixed. And then we can try it on some "smaller" repos, e.g. Cargo or Rust Analyzer.

Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Looks great! Feel free to merge with/without squashing.

@Urgau Urgau added this pull request to the merge queue Apr 7, 2025
Merged via the queue into rust-lang:master with commit ec68766 Apr 7, 2025
3 checks passed
@Urgau Urgau deleted the relink-handler branch April 7, 2025 16:33
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.

5 participants