-
Notifications
You must be signed in to change notification settings - Fork 105
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: encoding options added #359
Conversation
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.
I like adding the Encoder options.
I don't accept PRs without tests, as subsequent PRs to the same code often end up breaking or reverting that feature. Take a look at this test as a possible starting point:
Line 327 in 1d9fc9c
func TestEncodePartContentBinary(t *testing.T) { |
Looks pretty good to me, not sure if you're ready for review since it's still marked as draft? |
James, I wanted to add more tests after the holidays. But if you think that's enough, then I've changed the request status. |
Signed-off-by: James Hillyerd <[email protected]>
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.
Made a tweak to the test to check a bit of the encoded content.
James, please look at the proposed changes. #358