Skip to content

Conversation

@hyuma
Copy link

@hyuma hyuma commented Feb 4, 2020

To support Windows.
Without this fix, UnicodeDecodeError: 'cp932' codec can't decode byte 0xef in position 515: illegal multibyte sequence will occor when pip install

@hyuma hyuma requested a review from rizdaprasetya February 4, 2020 04:44
@hyuma
Copy link
Author

hyuma commented Feb 4, 2020

(this is bugfix)

@rizdaprasetya
Copy link
Collaborator

Nice, thanks for the contribution! @hyuma
But according to our check few months ago it will break compatibility with Python 2.7
rizdaprasetya#3

The best way to fix Windows compatibility and retain Python 2.7 compatibility is by following this sample setup.py file:
https://github.com/pypa/sampleproject/blob/master/setup.py#L8-L24

Would you like to try to add that?

Copy link
Collaborator

@rizdaprasetya rizdaprasetya left a comment

Choose a reason for hiding this comment

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

Please follow: #12 (comment)

@mashanz
Copy link
Contributor

mashanz commented Oct 21, 2021

Hi @rizdaprasetya , should we just make release version/branch/tag for backward compatibility?

  1. let say, release the un merged branch with version/branch/tag 1.0.0 (compatibility with python 2)
  2. and merge the branch and release it with version/branch/tag 2.0.0 and throw everything about python 2 compatibility
  3. with this way, it will made easier to track version and compatibility like new version of python or new version of dependencies

also the other reason is python 2 already dead almost 2 years ago. though?

@rizdaprasetya
Copy link
Collaborator

@mashanz correct, thanks for notifying, sorry we haven't review this for a while. Agree with your suggestion, esp. since python 2 is no longer supported officially. we'll use better version tagging to handle such compatibility concerns.

Also if you see any more compatibility issue to latest/current python version feel free to let us know by opening issue/PR. Cheers!

- for better compatibility across OS (esp Windows) & latest official python version (Python 3)
- may not work on Python 2 anymore
Copy link
Collaborator

@rizdaprasetya rizdaprasetya left a comment

Choose a reason for hiding this comment

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

Changed the setup.py implementation to follow official pypa sample project. It should have better compatibility with Windows OS & Python 3 now.

Thanks for your contribution @hyuma & @mashanz! 😃
Will merge now, and release as new version soon. Thanks

@rizdaprasetya rizdaprasetya merged commit 5f91625 into Midtrans:master Oct 25, 2021
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.

3 participants