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

Fix htmlbody encoding for hebrew text #118

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

santoshghimire
Copy link

Fix decoding for binary type

Fixes #117

@santoshghimire santoshghimire marked this pull request as ready for review June 1, 2021 12:05
@jugmac00
Copy link
Collaborator

jugmac00 commented Jun 1, 2021

Thanks for your contribution.

Could you please add a test case, which would fail without your fix?

@santoshghimire
Copy link
Author

@jugmac00 sure. Working on it. Thanks

@jugmac00
Copy link
Collaborator

jugmac00 commented Jun 2, 2021

@petri @santoshghimire is a first time contributor, and thus you need to enable the PR to run. GitHub has introduced this restriction to counter bit coin miners who abused CI. I do not have sufficient rights to do this - though I could merge, which is kinda odd.

@petri
Copy link
Member

petri commented Jun 3, 2021

I noticed our CI & CodeQL actions had stopped running due to "inactivity". I enabled them. Not sure if that's what you refer to? If not, where do I enable this PR to run then? I looked around but could not see anything.

@jugmac00
Copy link
Collaborator

jugmac00 commented Jun 4, 2021

I noticed our CI & CodeQL actions had stopped running due to "inactivity". I enabled them. Not sure if that's what you refer to? If not, where do I enable this PR to run then? I looked around but could not see anything.

The "inactivity" stop, which I think is a very bad term for a stable library, is something unrelated to the crypto restriction.

@petri, have a look at the screenshots at https://github.blog/2021-04-22-github-actions-update-helping-maintainers-combat-bad-actors/#new-features-to-help-protect-maintainers

I hope this helps.

Maybe we encounter some other glitch.

In this case it could help when @santoshghimire creates an empty commit and pushes it to trigger CI again:

git commit --allow-empty -m "Trigger CI"

@petri
Copy link
Member

petri commented Jun 4, 2021

Ok the empty commit did the trick. Weird the approval prompt needed that...

@jugmac00
Copy link
Collaborator

jugmac00 commented Jun 6, 2021

@santoshghimire As my local test suite on current master works, your changes seem to have introduced a couple of test failures. Could you have a look at the problems?

@santoshghimire
Copy link
Author

@jugmac00 sure. I am on it.

@santoshghimire
Copy link
Author

santoshghimire commented Jun 7, 2021

@jugmac00 I see tox does not support py2.7 because pytest-console-scripts only supports python 3+? I am trying to run tests for python2.7 too for 1.3.1 version

@petri
Copy link
Member

petri commented Jun 7, 2021

We have dropped Python2 support as you can see from the package description.

@jugmac00
Copy link
Collaborator

jugmac00 commented Jun 7, 2021

Additionally, we should add a python_requires in our setup.py.

see #119

@petri
Copy link
Member

petri commented Jun 7, 2021

@santoshghimire do you need Python2 support for some reason?

@santoshghimire
Copy link
Author

@petri yes. I have been using 1.3.1 version for python 2.7

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.

htmlbody for Hebrew text (iso-8859-8-i encoding) is incorrect
3 participants