regro / rever

Releaser of Versions
https://regro.github.io/rever-docs/
BSD 3-Clause "New" or "Revised" License
74 stars 34 forks source link

FIX: `repo.create_fork` #126

Closed CJ-Wright closed 6 years ago

CJ-Wright commented 6 years ago

Attn: @justcalamari

scopatz commented 6 years ago

Can you explain please? I don't understand how this works or why it is needed

CJ-Wright commented 6 years ago

One only need supply an arg to create_repo if one is cloning into an org account. Since username is not an org account, this errors if a user doesn't have a fork.

scopatz commented 6 years ago

That doesn't make much sense to me. A user should not be required to have a for to create a fork. Can you provide a traceback? Also sometimes we may fork into an org, and so this parameter is sometimes needed. It seems like the fix is more sophisticated.

scopatz commented 6 years ago

*have a fork to create a fork

CJ-Wright commented 6 years ago
activity-start:conda_forge:starting activity conda_forge
Fork doesn't exist creating feedstock fork...
activity-error:conda_forge:activity failed with execption:
Traceback (most recent call last):
  File "/home/christopher/dev/rever/rever/activity.xsh", line 62, in __call__
    self.func(*args, **kwargs)
  File "/home/christopher/dev/rever/rever/activities/conda_forge.xsh", line 147, in _func
    repo.create_fork(username)
  File "/home/christopher/mc/envs/regro/lib/python3.6/site-packages/github3/decorators.py", line 38, in auth_wrapper
    return func(self, *args, **kwargs)
  File "/home/christopher/mc/envs/regro/lib/python3.6/site-packages/github3/repos/repo.py", line 595, in create_fork
    json = self._json(resp, 202)
  File "/home/christopher/mc/envs/regro/lib/python3.6/site-packages/github3/models.py", line 100, in _json
    if self._boolean(response, status_code, 404) and response.content:
  File "/home/christopher/mc/envs/regro/lib/python3.6/site-packages/github3/models.py", line 121, in _boolean
    raise GitHubError(response)
github3.models.GitHubError: 422 'CJ-Wright' is the login for a user account. You must pass the login for an organization account.
rewinding to dc35e61b49d064c55c21e6811a4440c186e7b528
CJ-Wright commented 6 years ago

The issue is that one has a fork then we bypass the fork creation step. If not we fail because the user isn't an org (due to the signature of the method).

scopatz commented 6 years ago

What version of github3.py are you using?

CJ-Wright commented 6 years ago

github3.py 0.9.6 py36_0 conda-forge

scopatz commented 6 years ago

hmm ok that is the correct version...

CJ-Wright commented 6 years ago

@scopatz can I get some more review? I've implemented an env var so people can specify an org to fork to if needed. CC: @jakirkham I think this may address the issue you were facing with rever and conda-smithy.

CJ-Wright commented 6 years ago

Thank you very much!

jakirkham commented 6 years ago

Thanks all. :)

CJ-Wright commented 6 years ago

@jakirkham I think that if you are using xonsh you can set the $CONDA_FORGE_FORK_ORG in your xonsh shell and then call rever and rever will respect it. That way you don't need to put it in the rever.xsh that everyone uses.

jakirkham commented 6 years ago

Will it also ask me for this info (or can I have it ask for this info)?

scopatz commented 6 years ago

@jakirkham - what do you mean exactly? Like interactively when you run rever?

jakirkham commented 6 years ago

Yes, when running rever <some version> with conda-smithy would it ask for this information?

scopatz commented 6 years ago

No, not by default. It would read the value of $CONDA_FORGE_FORK_ORG from the environment. However, in the rever.xsh file you could add a line that looked like:

$CONDA_FORGE_FORK_ORG = input('Feedstock Fork Org: ').strip()
scopatz commented 6 years ago

Though that is an interesting idea to be able to flag certain parameters as interactively input...

jakirkham commented 6 years ago

So was wondering that as it seemed much of information did get pulled in interactively (e.g. GitHub handle, etc.)

scopatz commented 6 years ago

Ahh yeah, but all of that interactive stuff is info that is unqiue to the user running rever, rather than being properties of the project itself

jakirkham commented 6 years ago

Ah ok. Thanks for clarifying.

So I think this could be a user setting, right? As it would always be dependent on the user running rever to determine where the fork should live. Is it possible to have defaults for these (e.g. assume the fork can be placed under user repos)? Or am I misunderstanding user settings (e.g. do they apply to other projects too)? Still very new to rever.

Anyways we probably shouldn't think to hard about this. My use case is definitely a special snow flake case. Having an environment variable should be fine. Might add a note in the README so I don't forget. :)

scopatz commented 6 years ago

Is it possible to have defaults for these (e.g. assume the fork can be placed under user repos)

yes, that is exactly the default right now.

So I think this could be a user setting, right? As it would always be dependent on the user running rever to determine where the fork should live

We can certainly it to be a user setting. It would probably be nice to have a user config file that gets loaded that specifies certain parameters for the user..

Thanks for banging on it!

CJ-Wright commented 6 years ago

@scopatz maybe put something under .conf which we source in rever's execution?

scopatz commented 6 years ago

To be open desktop compliant, it would have to be in ~/.config/rever. But, yeah, we could add a rc.xsh file to that directory that gets sourced after the rever.xsh file is sourced

jakirkham commented 6 years ago

If you haven't already thoughts/done lots with config stuff, you may find this comment a good read.

scopatz commented 6 years ago

Yeah that is a great comment!