-
Notifications
You must be signed in to change notification settings - Fork 423
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
adding some fixes to cpan skeleton #1704
Conversation
@@ -174,10 +174,11 @@ def skeletonize(packages, repo, output_dir=".", version=None, recursive=False, | |||
sources into expected conda recipe format.""" | |||
|
|||
version = getattr(config, "version", version) | |||
#I dont now if these do anything |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@msarahan , do these do anything? version, outdir, and recursive are not in kwargs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea, I'm sorry. I'm glad you put them there in main_skeleton.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm reasonably certain they aren't doing anything, but I feel nervous about getting rid of things.
@@ -174,10 +174,11 @@ def skeletonize(packages, repo, output_dir=".", version=None, recursive=False, | |||
sources into expected conda recipe format.""" | |||
|
|||
version = getattr(config, "version", version) | |||
#I dont now if these do anything |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea, I'm sorry. I'm glad you put them there in main_skeleton.
if version: | ||
kwargs.update({'version': version}) | ||
if recursive: | ||
kwargs.update({'version': version}) | ||
kwargs.update({'recursive': recursive}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch.
Looks like you need to catch a CondaRuntimeError for conda 4.2.x. Otherwise fine. Thanks! Please sign the CLA, too, if you can. If you need to consult your legal department, I can forward that info to the appropriate people here. |
@msaharn, not a problem, but I already signed a CLA with conda core. Are they separate projects? How is the little one? |
@jerowe thanks - you only need to have signed one of the CLAs. Eric is fat and happy most of the time. Can't ask for more. =) Thanks for asking. |
@jerowe fixed things up. Sorry, they were a bit of a mess. Needed to pull in CondaError from conda 4.2, not just 4.3. This should pass CI now. Will merge when it does. |
Hooray! it worked! merging. |
Thanks, @msarahan! |
Hi there, thank you for your contribution! This pull request has been automatically locked because it has not had recent activity after being closed. Please open a new issue or pull request if needed. Thanks! |
@msarahan, here is my PR for #1675.
Mostly, the metacpan api url needed to be updated, but there were a few other fixes.