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

Avoid multiple PRs suggesting the same change #399

Open
jishnub opened this issue Oct 4, 2021 · 12 comments
Open

Avoid multiple PRs suggesting the same change #399

jishnub opened this issue Oct 4, 2021 · 12 comments
Labels

Comments

@jishnub
Copy link

jishnub commented Oct 4, 2021

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.

@DilumAluthge
Copy link
Member

Seems like a bug. @mattBrzezinski @fchorney

@fchorney
Copy link
Collaborator

fchorney commented Oct 4, 2021

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
Copy link
Collaborator

fchorney commented Oct 4, 2021

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.

@DilumAluthge
Copy link
Member

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.

@fchorney
Copy link
Collaborator

fchorney commented Oct 4, 2021

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.

@DilumAluthge
Copy link
Member

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.

@jishnub
Copy link
Author

jishnub commented Oct 5, 2021

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?

@DilumAluthge
Copy link
Member

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

@mattBrzezinski
Copy link
Member

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

@mattBrzezinski
Copy link
Member

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

@DilumAluthge
Copy link
Member

DilumAluthge commented Oct 5, 2021

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.

@jishnub
Copy link
Author

jishnub commented Oct 5, 2021

Interesting, this certainly seems reasonable, thanks!

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

No branches or pull requests

4 participants