search

ftpsolutions / python-third-party-license-file-generator

A tool that walks your Python project's requirements and gathers the third party licenses for you
MIT License
13 stars 5 forks source link

Changes to support Windows #3

Closed fredvisser closed 5 years ago

fredvisser commented 5 years ago
  1. subprocess.Popen() has platform specific arguments. Added a platform check before calling a Windows compatible version of the call.
  2. Replaced a manual path join with a os.path.join call (and removed unnecessary /)
  3. Added encoding='utf-8', errors='ignore' parameters to the file.open call to resolve issues where some files contained un-parseable characters.

I've done basic testing to confirm that it works correctly on Windows and macOS.

initialed85 commented 5 years ago

Thanks for the PR!

Looks good to me overall- I tested with that changed import line (see comment in site_packages.py and it all still works on MacOS.

Another request- I seem to have overlooked the PyYAML requirement in setup.py under the install_requires part; are you able to fix that up on my behalf?

fredvisser commented 5 years ago

Great feedback!

  1. Fixed the PyYAML requirement (and bumped it to 3.13 to resolve an install issue I saw on 3.12).
  2. I've added the new import statement (from codecs import open).

One minor note from testing with 2.7 and 3.7 – with 2.7 the license summary looks like:

# 2.7
Apache-2.0
        u'python-dateutil' by u'Gustavo Niemeyer <gustavo@niemeyer.net>' (u'https://dateutil.readthedocs.io')
        u'requests' by u'Kenneth Reitz <me@kennethreitz.org>' (u'http://python-requests.org')
# 3.7
Apache-2.0
        'python-dateutil' by 'Gustavo Niemeyer <gustavo@niemeyer.net>' ('https://dateutil.readthedocs.io')
        'requests' by 'Kenneth Reitz <me@kennethreitz.org>' ('http://python-requests.org')
initialed85 commented 5 years ago

Great feedback!

  1. Fixed the PyYAML requirement (and bumped it to 3.13 to resolve an install issue I saw on 3.12).
  2. I've added the new import statement (from codecs import open).

One minor note from testing with 2.7 and 3.7 – with 2.7 the license summary looks like:

# 2.7
Apache-2.0
        u'python-dateutil' by u'Gustavo Niemeyer <gustavo@niemeyer.net>' (u'https://dateutil.readthedocs.io')
        u'requests' by u'Kenneth Reitz <me@kennethreitz.org>' (u'http://python-requests.org')
# 3.7
Apache-2.0
        'python-dateutil' by 'Gustavo Niemeyer <gustavo@niemeyer.net>' ('https://dateutil.readthedocs.io')
        'requests' by 'Kenneth Reitz <me@kennethreitz.org>' ('http://python-requests.org')

Hmm- It's really just the user-facing summary with the main output being the generated licenses I guess.

I don't mind if you don't mind!

fredvisser commented 5 years ago

I don't mind if you don't mind!

I don't mind!

lowaa commented 5 years ago

I've also tested it on MAC OS and it womm 👍 Thanks for the update!