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

incorrect PR references (possibly limited to patatrack prs?) #31952

Open
davidlange6 opened this issue Oct 27, 2020 · 8 comments
Open

incorrect PR references (possibly limited to patatrack prs?) #31952

davidlange6 opened this issue Oct 27, 2020 · 8 comments

Comments

@davidlange6
Copy link
Contributor

I noticed this while browsing cmssw code

image

of course #557 in the cmssw repo has nothing to with this code change. I presume its instead from the patatrack repository instead. Can we get the crossreferences to be correct at least for future PRs?

@cmsbuild
Copy link
Contributor

A new Issue was created by @davidlange6 David Lange.

@Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@makortel
Copy link
Contributor

assign core

@cmsbuild
Copy link
Contributor

New categories assigned: core

@Dr15Jones,@smuzaffar,@makortel you have been requested to review this Pull request/Issue and eventually sign? Thanks

@makortel
Copy link
Contributor

I suppose this is a "feature" whenever PRs in forks of cms-sw/cmssw are merged there, and then the branch containing such merges is PR'd+merged in the main cms-sw/cmssw. I have seen these also before the Patatrack PRs.

Fixing the references to point the correct fork would require rewriting the commit messages. A script itself to do that should be straightforward (assuming all the references are from a single fork). I'm less certain how much that could be reliably automated.

@makortel
Copy link
Contributor

Here is a similar observation elsewhere isaacs/github#1072 .

@makortel
Copy link
Contributor

Let me also add @fwyzard explicitly to comment

Can we get the crossreferences to be correct at least for future PRs?

@fwyzard
Copy link
Contributor

fwyzard commented Oct 27, 2020

Mhm, I see.

I can fix the commit history it for all the Patatrack PRs that are still open.

I guess we don't want to re-write the history to fix the past commits, though ?

@makortel
Copy link
Contributor

I can fix the commit history it for all the Patatrack PRs that are still open.

Thanks Andrea!

I guess we don't want to re-write the history to fix the past commits, though ?

My feeling is the same, i.e. these would not be big-enough deal to justify the trouble of rewriting the history of master.

One option towards preventing such PRs to be merged in the future would be to detect if any of the commit messages contain a PR/issue reference, and ask the author and reviewers to double-check those. I'm not sure if we could automatically identify non-cms-sw/cmssw references though, and just adding a message by @cmsbot could be easily missed.

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

No branches or pull requests

4 participants