Skip to content

Comments

Improve MeFilter#271

Closed
ukleinek wants to merge 2 commits intoafewmail:masterfrom
ukleinek:improve-mefilter
Closed

Improve MeFilter#271
ukleinek wants to merge 2 commits intoafewmail:masterfrom
ukleinek:improve-mefilter

Conversation

@ukleinek
Copy link
Contributor

Note this is completely untested, so please look deeply before merging :-)

ukleinek added 2 commits May 27, 2020 19:01
The me_tag option does nothing that couldn't be done with the tags option
that works on all Filters.

Recommend using tags instead of me_tag in the documentation and drop the
custom handle_message. Add some code to make existing rules work as
expected.

As a side-effect this makes

	[MeFilter]
	tags = -something

work as expected. It used to do nothing, now it removes the something tag.
Instead of accepting only tags_blacklist just pass everything but
me_tags to the super class's init function. This way the message can be
overwritten in the config which up to now yields an exception:

	TypeError: __init__() got an unexpected keyword argument 'message'
@codecov
Copy link

codecov bot commented May 27, 2020

Codecov Report

❌ Patch coverage is 16.66667% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.40%. Comparing base (2476428) to head (c6b3da5).
⚠️ Report is 94 commits behind head on master.

Files with missing lines Patch % Lines
afew/filters/MeFilter.py 16.66% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #271      +/-   ##
==========================================
- Coverage   40.51%   40.40%   -0.11%     
==========================================
  Files          30       30              
  Lines         980      980              
==========================================
- Hits          397      396       -1     
- Misses        583      584       +1     
Flag Coverage Δ
unittests 40.40% <16.66%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@GuillaumeSeren
Copy link
Collaborator

Hello @ukleinek
First and foremost thank you for the interest and you submited patches on the project 👍

Note this is completely untested, so please look deeply before merging :-)

Yes actually there was some significant progress on the tests side of the project,
so creating a test for the MeFilter seems a fair preriquisite to merge this PR.

I will comment the diff.

@GuillaumeSeren
Copy link
Collaborator

Also ping @aidecoe original author of this filter

@aidecoe
Copy link
Contributor

aidecoe commented May 31, 2020

Hi ukleinek. What is the improvement? Or is it just a code refactor?

@ukleinek ukleinek closed this by deleting the head repository Feb 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants