CompatHelper.jl icon indicating copy to clipboard operation
CompatHelper.jl copied to clipboard

Avoid multiple PRs suggesting the same change

Open jishnub opened this issue 2 years ago • 12 comments

See https://github.com/jishnub/SphericalHarmonics.jl/pulls?q=is%3Aopen+is%3Apr, where there appears to be a new PR everyday suggesting the same change. Not sure what brought this on.

jishnub avatar Oct 04 '21 08:10 jishnub

Seems like a bug. @mattBrzezinski @fchorney

DilumAluthge avatar Oct 04 '21 12:10 DilumAluthge

huh very interesting. Looks like this started on September 17th, but CompatHelper itself hasn't changed in any significant way since August. I can't immediately see why this would be happening. The titles are all exactly the same, and thus should theoretically stop the duplication. I don't see any errors in the log output either. I don't see any significant changes in the SphericalHarmonics code either in that time frame.

fchorney avatar Oct 04 '21 14:10 fchorney

The only installed package differences between the CompatHelper runs on the 15th and the 17th are that JSON2 got upgraded from 3.2 to 3.3. I don't see any other differences there.

fchorney avatar Oct 04 '21 14:10 fchorney

Actually, I just noticed that https://github.com/jishnub/SphericalHarmonics.jl/pulls?q=is%3Aopen+is%3Apr is a fork.

CompatHelper is not intended to be run on forks; it is intended to be run on the upstream repository.

@jishnub I recommend that you go to https://github.com/jishnub/SphericalHarmonics.jl/settings/actions and disable GitHub Actions on your fork.

DilumAluthge avatar Oct 04 '21 14:10 DilumAluthge

oh good find. I didn't notice that! The reason it's duplicating them is because we don't check the PR titles for forks. Perhaps a better error message might work in this case, such that we detect if we're in a fork, print an error message and not run.

fchorney avatar Oct 04 '21 14:10 fchorney

Perhaps a better error message might work in this case, such that we detect if we're in a fork, print an error message and not run.

We'd want to make sure that we exit with a nonzero status code in this case. That way, the CompatHelper runs show up as failed (red) in the logs.

DilumAluthge avatar Oct 04 '21 14:10 DilumAluthge

Thanks for the find!

Unfortunately in this case the upstream is unregistered and not maintained, and my fork is the one that is registered. Is there no way to get it to work on this repo?

jishnub avatar Oct 05 '21 05:10 jishnub

@fchorney @mattBrzezinski We'd need to change our logic for excluding PRs from forks. We could keep this check if the repo that's running CompatHelper is not a fork. But we'd have to skip that check if and only if the repo that's running CompatHelper is a fork.

DilumAluthge avatar Oct 05 '21 16:10 DilumAluthge

I can start looking into this now, I have some free time.

mattBrzezinski avatar Oct 05 '21 16:10 mattBrzezinski

Overview

So... this is a really weird case and there are a few ways we can handle resolving the issue here. To clarify the problem, we have a Julia package which is a fork of a fork that has been registered in the General registry.

We do not run CompatHelper on forks of repos, because we've been running under the assumption that people follow a normal workflow with forks. Which usually is; fork off of origin, creating some changeset, and create a merge request back into origin.

Solutions

  1. Implement some functionality into CompatHelper.jl to allow for this behaviour to work. I personally do not see the value in creating this functionality and adding complexity for a one-off case.

  2. Contact GitHub support to apparently transfer as forked project into its own standalone project.

  3. Make merge requests to the upstream origins, and then update the General registry to point to the new location.

  4. Have upstream origin owners transfer ownership to @jishnub for the project.

  5. Create a new standalone project, copy the code into it, juggle the names of the projects temporarily. This step is a bit dangerous as it would break the registry for a time being from my understanding.

I think the solution here is to see how responsive the upstream owners are and if they are willing to either transfer / take in PRs. Alternatively, just cutting an issue with GitHub support would be quite simple however I'm not sure how quick to respond they would be.

If you want this fixed ASAP, I think solution 5 is the play. I'd say just have it planned out before hand as you'll basically need to:

  • Create a new repository called foobar
  • Transfer in the code and old tags
  • Delete the fork of SphericalHarmonics.jl
  • Rename the foobar project to SphericalHarmonics.jl

mattBrzezinski avatar Oct 05 '21 17:10 mattBrzezinski

2. Contact GitHub support to apparently transfer as forked project into its own standalone project.

@jishnub Given that your repo is the registered repo, I would recommend going with option 2. It can be confusing for users when they are looking for the registered repo but they find that it is a fork.

DilumAluthge avatar Oct 05 '21 17:10 DilumAluthge

Interesting, this certainly seems reasonable, thanks!

jishnub avatar Oct 05 '21 17:10 jishnub