-
-
Notifications
You must be signed in to change notification settings - Fork 46
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
feat: implement WithExactWord to allow stricter matches #107
base: master
Are you sure you want to change the base?
Conversation
goaway_test.go
Outdated
{ | ||
name: "With Custom Dictionary", | ||
profanityDetector: NewProfanityDetector().WithCustomDictionary(DefaultProfanities, DefaultFalsePositives, DefaultFalseNegatives), | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{ | |
name: "With Custom Dictionary", | |
profanityDetector: NewProfanityDetector().WithCustomDictionary(DefaultProfanities, DefaultFalsePositives, DefaultFalseNegatives), | |
}, | |
{ | |
name: "With Custom Dictionary", | |
profanityDetector: NewProfanityDetector().WithCustomDictionary(DefaultProfanities, DefaultFalsePositives, DefaultFalseNegatives), | |
}, |
Was this a typo? It's a duplicate of the scenario above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I meant to have it include coverage of the new code by setting it to use exact words for one of them. pushed that fix in bf6efc8
goaway_test.go
Outdated
accept_sentences := []string{ | ||
"I'm an analyst", | ||
} | ||
reject_sentences := []string{"Go away, ass."} | ||
tests := []struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accept_sentences := []string{ | |
"I'm an analyst", | |
} | |
reject_sentences := []string{"Go away, ass."} | |
tests := []struct { | |
accept_sentences := []string{ | |
"I'm an analyst", | |
} | |
reject_sentences := []string{"Go away, ass."} | |
tests := []struct { |
Go uses camelCase instead of snake_case - could you make the changes to the other parts of your code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in bf6efc8, thank you! the perils of switching between languages and getting my styles mixed up 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for the feedback @TwiN ! I've updated the PR with it. sorry it took a while—shortly after the feedback I went on holiday, but I should be able to have more timely iteration now if this needs any further work.
goaway_test.go
Outdated
{ | ||
name: "With Custom Dictionary", | ||
profanityDetector: NewProfanityDetector().WithCustomDictionary(DefaultProfanities, DefaultFalsePositives, DefaultFalseNegatives), | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I meant to have it include coverage of the new code by setting it to use exact words for one of them. pushed that fix in bf6efc8
goaway_test.go
Outdated
accept_sentences := []string{ | ||
"I'm an analyst", | ||
} | ||
reject_sentences := []string{"Go away, ass."} | ||
tests := []struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in bf6efc8, thank you! the perils of switching between languages and getting my styles mixed up 😅
Summary
This is a fix for #38 to filter only exact words.
I chose to implement it by having an alternate implementation only for the profanities list, where the rest is untouched so false positives and false negatives will remain working as expected; it seems right to me that a false negative should not change from exact word matches.
I have one test in here. I could see a use for more, but comments/feedback in that area would be appreciated so I can calibrate what sort of test coverage you're aiming for.
Checklist
README.md
, if applicable.