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

added urwid recipe #316

Merged
merged 3 commits into from
Apr 12, 2016
Merged

added urwid recipe #316

merged 3 commits into from
Apr 12, 2016

Conversation

scopatz
Copy link
Member

@scopatz scopatz commented Apr 12, 2016

Didn't see it yet...

@conda-forge-admin
Copy link
Contributor

Hi! This is the friendly conda-forge-admin automated user.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/urwid) and found it was in an excellent condition.

@@ -0,0 +1,2 @@
@echo print('An example') > %SP_DIR%\example.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intended?

@scopatz
Copy link
Member Author

scopatz commented Apr 12, 2016

Also, how does one disable windows packages such as for this?

@jakirkham
Copy link
Member

Ah ok, now it makes sense. Happy to show you.

md5: 2e1a005cb31368fe21bfeba2d6ad5a5c

build:
number: 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply add the following after the build number in the build section.

skip: true  # [win]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh Ok. I think I remember seeing this somewhere, but some better docs for this would be nice since I couldn't find it when I went looking for it :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we definitely need docs in general. This came up in discussion last week.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as it is on the docket!

@scopatz
Copy link
Member Author

scopatz commented Apr 12, 2016

Ok @jakirkham I think I have made all of the requested changes!


build:
number: 0
skip: true # [win]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have also been folding one line scripts into the build section. If the package were using distutils only, it would look like this.

script: python setup.py install

However, as it uses setuptools, we add this.

script: python setup.py install --single-version-externally-managed --record record.txt

The additional arguments stop setuptools from handling some stuff for us that causes conda-build problems.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sort of seems like the example meta.yaml should contain all of these options and explanations as comments. That way, if you look at the example or just copy it over, it sort of serves as a walkthrough.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, made this change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. We probably should have a few examples to demonstrate common use cases. For instance, a simple pure Python program, a C/C++ program that is maybe UNIX only, and maybe a cross platform C/C++ program (Windows too). I think these are common cases that we answer very similar questions about.

Adding the linter bot has helped some by directing people to these issues. However, you are right we should simplify for people from the outset. It will save reviewer time and leave contributors, hopefully, more satisfied.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this issue ( #318 ) to follow up on improving and adding more examples. Please feel free to share more thoughts on it.

@jakirkham
Copy link
Member

One more fix and then we should be good.

@jakirkham
Copy link
Member

LGTM. Thanks @scopatz.

@jakirkham jakirkham merged commit 7722a54 into conda-forge:master Apr 12, 2016
@scopatz
Copy link
Member Author

scopatz commented Apr 12, 2016

Thanks!

@jakirkham
Copy link
Member

Of course.

Thanks for your feedback on the process. Sometimes its difficult to see the forest from the trees.

@scopatz
Copy link
Member Author

scopatz commented Apr 12, 2016

No problem! Thanks for all of your hard work. Happy to help in any way I can!

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

Successfully merging this pull request may close these issues.

None yet

3 participants