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

Provide more help if lessc is not found. #5633

Merged
merged 1 commit into from Apr 16, 2014

Conversation

ahmadia
Copy link
Contributor

@ahmadia ahmadia commented Apr 16, 2014

A slightly more useful error message.

I used a ValueError to match the other exceptions, but I think OSError might be a more appropriate exception.

Addresses: #5632

@ahmadia
Copy link
Contributor Author

ahmadia commented Apr 16, 2014

And this is minor but a comment in the file indicates lessc@1.5.0 is excluded by the version check, but it isn't.

@minrk
Copy link
Member

minrk commented Apr 16, 2014

Do you want to fix the check for 1.5? It should be just changing > to >=.

@ahmadia
Copy link
Contributor Author

ahmadia commented Apr 16, 2014

@minrk - I don't understand. distutils.LooseVersion passes this test:

 if V(less_version) > V(max_less_version):

for less_version and max_less_version both equal to 1.5.0, there's an implicit = in there. I can adjust the documentation in the file to match that, unless 1.5.0 is a problem, in which case the logic will need to be fixed.

@minrk
Copy link
Member

minrk commented Apr 16, 2014

1.5.0 should be excluded

@ahmadia
Copy link
Contributor Author

ahmadia commented Apr 16, 2014

Thanks, I'll figure out what's going on and update the PR.

@ahmadia
Copy link
Contributor Author

ahmadia commented Apr 16, 2014

Oh, I'm an idiot. Thanks.

* lessc@1.5.0 now invalid
* more help if lessc not found
@minrk
Copy link
Member

minrk commented Apr 16, 2014

lessc output changes dramatically across versions, so pinning the version helps to avoid massive diffs with small CSS changes. We will probably pin a later version during the 3.0 cycle.

@minrk minrk added this to the 2.1 milestone Apr 16, 2014
@ahmadia
Copy link
Contributor Author

ahmadia commented Apr 16, 2014

@minrk - I redid the commit, this is ready for review again.

minrk added a commit that referenced this pull request Apr 16, 2014
Provide more help if lessc is not found.
@minrk minrk merged commit c064bfe into ipython:master Apr 16, 2014
@minrk
Copy link
Member

minrk commented Apr 16, 2014

thanks

@ahmadia ahmadia deleted the improve_no_lessc_error branch April 16, 2014 04:00
minrk added a commit that referenced this pull request Apr 16, 2014
A slightly more useful error message.

I used a ValueError to match the other exceptions, but I think OSError might be a more appropriate exception.

Addresses: #5632
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Provide more help if lessc is not found.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants