System sass #219
System sass #219
Conversation
setup.py
Outdated
| else: | ||
| link_flags = ['-fPIC', '-lstdc++'] | ||
|
|
||
| if system_sass: |
There was a problem hiding this comment.
instead of three if system_sass: blocks, can you combine these?
setup.py
Outdated
| from setuptools import Extension, setup | ||
|
|
||
| LIBSASS_SOURCE_DIR = os.path.join('libsass', 'src') | ||
| system_sass = os.environ.get('SYSTEMSASS', False) |
There was a problem hiding this comment.
this looks like SYSTEMS ASS to me :), how about SYSTEM_SASS?
asottile
dismissed
their stale review
This looks fine to me, curious to see what @dahlia says
|
It looks good to me. It's okay to merge if the builds succeed. |
setup.py
Outdated
| if tuple( | ||
| map(int, platform.mac_ver()[0].split('.')) | ||
| ) >= (10, 9): | ||
| ) >= (10, 9): |
There was a problem hiding this comment.
I think the correct response to this lint is to indent the map line by one more space instead of the paren
There was a problem hiding this comment.
Or save width a simpler and cleaner way:
macver = tuple(map(int, platform.mac_ver()[0].split('.')))
if macver >= (10, 9):
setup.py
Outdated
| if tuple( | ||
| map(int, platform.mac_ver()[0].split('.')) | ||
| ) >= (10, 9): | ||
| map(int, platform.mac_ver()[0].split('.')) |
There was a problem hiding this comment.
this should be indented 8 more spaces
There was a problem hiding this comment.
https://www.python.org/dev/peps/pep-0008/#indentation
notably the part where it says:
"More indentation included to distinguish this from the rest."
|
Thanks! |
|
This has been released as part of 0.13.3! |
3 participants
This PR implements solution for using system-installed libsass library to build the python sass extension.
This will help to get rid of some hacks/downstream-patches in Fedora and other distributions' packaging.