search

koodaamo / tnefparse

a TNEF decoding library written in python, without external dependencies
GNU Lesser General Public License v3.0
49 stars 37 forks source link

Fix htmlbody encoding for hebrew text #118

Open santoshghimire opened 3 years ago

santoshghimire commented 3 years ago

Fix decoding for binary type

Fixes https://github.com/koodaamo/tnefparse/issues/117

jugmac00 commented 3 years ago

Thanks for your contribution.

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

santoshghimire commented 3 years ago

@jugmac00 sure. Working on it. Thanks

jugmac00 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

jugmac00 commented 3 years ago

@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 commented 3 years ago

@jugmac00 sure. I am on it.

santoshghimire commented 3 years ago

@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 commented 3 years ago

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

jugmac00 commented 3 years ago

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

see #119

petri commented 3 years ago

@santoshghimire do you need Python2 support for some reason?

santoshghimire commented 3 years ago

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