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
Add IPython-SQL to conda-forge #1192
Conversation
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipes/ipython-sql:
|
@@ -0,0 +1,46 @@ | |||
{$ set version = "0.3.6" %} |
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.
Should be {% set version = "0.3.6" %}
.
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.
Small punctuation is our downfall again. 😆
Looks pretty good @sodre. Few things to fix up, but they are all pretty easy. Thanks for the proposed addition. |
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
Circle failure appears related to trying to compile pexpect python3 file. I think/hope newer versions of conda-build will fix this. Not sure if you want to try with 1.21.11 from source, package seems to not be available yet. |
Didn't |
let me know what/if you need me to change. |
md5: {{ md5 }} | ||
|
||
build: | ||
number: 0 |
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.
Well, if you want to just do Python 3.x, please add skip: true # [py2k]
. If we need Python 3.5 only, we may need to change py2k
to py35
. Once this is done, please drop the changes to .CI/build_all
from the PR.
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.
Guessing this is not needed now, but have forgotten. Maybe a reminder would be nice please. 😁
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipes/ipython-sql:
|
script: python setup.py install --single-version-externally-managed --record record.txt | ||
|
||
skip: | ||
true # [py2k] |
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.
So this should be in the build
section, it should be one line, and the selector should have at least 2 spaces before it.
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.
@jakirkham,
Thanks so much for helping me out on this. is there an easy way to run the lint checker on my side before committing?
76043b7
to
7cb8d10
Compare
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipes/ipython-sql:
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
build: | ||
number: 0 | ||
script: python setup.py install --single-version-externally-managed --record record.txt | ||
skip: true # [py2k] |
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.
We should be able to drop this skip
line as conda-build
version 1.21.11
is out and shouldn't run into the same issues.
Hmm...seems we are still running into this issue on CircleCI still, but not the other CIs. Will try rebuilding the Docker image. As for the other CIs, seems we may be running into some other sort of issue ( spyder-ide/spyder#3309 ). @ccordoba12, do you know anything about what is causing the |
Interestingly I don't run into this issue with |
Rebuilt the Docker container and now restarting CircleCI. |
@conda-forge/pygments, any ideas on this last error? |
As for the first error, could you please try updating |
@jakirkham, @Carreau said in spyder-ide/spyder#3309 that this problem was a bug in IPython 5.0, and that it'll be fixed in 5.1. |
Ah, ok, sorry for being dense. Thanks for clarifying, @ccordoba12. |
Yes, Sorry I'm trying to push 5.1 / 5.0.1 through the door. I'm doing the best I can. Sorry about that. |
No worries. Just trying to narrow down the cause. |
If it would help alleviate some pressure, @Carreau, we could try applying a patch that fixes the issue to our |
Ok, I had a deeper look and now I'm confused. Looking at the travis error, I have no clue why. The issue @ccordoba12 linked to have 2 issues. One with |
Was referring to the latter issue. Though we are using the latest |
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
Btw, I think it is failing because IPython has been spelled with a lower case i in a few places, while it is IPython |
Dang, the |
Must be GitHub's way of saying no more joking around. 😜 |
alright, it looks like the only issue left is with pygments for python 3.5, IPython 5 and win[64,32]. |
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
- Win and Py35 require IPython<5
@jakirkham, It looks like the package is now ready to go. |
- sql | ||
|
||
about: | ||
home: pypi.python.org/pypi/ipython-sql |
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.
Might change to https://pypi.python.org/pypi/ipython-sql
|
||
about: | ||
home: https://pypi.python.org/pypi/ipython-sql | ||
license: MIT |
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.
Ideally we would include the license file. However, they don't provide one. 😞
Have raised issue ( catherinedevlin/ipython-sql#58 ) to track this. Could we please add a comment here to add the license_file
once they have provided one?
We need to add the MIT license file when catherinedevlin/ipython-sql#58 closes.
Thanks everyone for your help getting this one worked out. Thanks @sodre for contributing it. |
Soon you should get an email that invites you to join conda-forge, @sodre. Once accepted you will be added to a team with the same name as this recipe. Those will give you permissions on the feedstock (repo) for this recipe. Make sure when proposing any change that you go through the typical GitHub workflow of forking the feedstock and making changes in your fork that you PR back. Once merged CIs will build and deploy any changes you make. Please let us know if you have any questions and welcome to conda-forge. |
Thanks! I will keep an eye out for the License fix. Best,
|
Please add iPython-SQL to conda-forge.