tnefparse icon indicating copy to clipboard operation
tnefparse copied to clipboard

Fix htmlbody encoding for hebrew text

Open santoshghimire opened this issue 3 years ago • 13 comments

Fix decoding for binary type

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

santoshghimire avatar Jun 01 '21 12:06 santoshghimire

Thanks for your contribution.

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

jugmac00 avatar Jun 01 '21 12:06 jugmac00

@jugmac00 sure. Working on it. Thanks

santoshghimire avatar Jun 01 '21 12:06 santoshghimire

@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.

jugmac00 avatar Jun 02 '21 11:06 jugmac00

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.

petri avatar Jun 03 '21 20:06 petri

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"

jugmac00 avatar Jun 04 '21 05:06 jugmac00

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

petri avatar Jun 04 '21 14:06 petri

@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?

jugmac00 avatar Jun 06 '21 07:06 jugmac00

@jugmac00 sure. I am on it.

santoshghimire avatar Jun 06 '21 09:06 santoshghimire

@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

santoshghimire avatar Jun 07 '21 05:06 santoshghimire

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

petri avatar Jun 07 '21 05:06 petri

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

see #119

jugmac00 avatar Jun 07 '21 06:06 jugmac00

@santoshghimire do you need Python2 support for some reason?

petri avatar Jun 07 '21 15:06 petri

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

santoshghimire avatar Jun 08 '21 05:06 santoshghimire