-
Notifications
You must be signed in to change notification settings - Fork 78
Conversation
@@ -256,6 +257,9 @@ func TestAttachmentOnly(t *testing.T) { | |||
} | |||
|
|||
testMessage(t, m, 0, want) | |||
if err := teardownFile("/tmp/test.pdf"); err != nil { | |||
panic(err) |
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.
It's generally not a good practice to panic unless absolutely necessary. Have a look at using t.Errorf("teardown: %s", err)
instead.
@Gamma169 Thanks for making time for this over your weekend. On further consideration, I don't think we'll be able to benefit from this improvement without sacrificing usability/features somewhere else:
I was considering simply adding an |
@ivy why? this is a good patch, got a lot of stuff in there.. at least create the v3 branch.. |
@ivy serious do not reject.. accept everything as a possibility.. and id u do that.. then u have a whole heap behind u,, Thing u a good "leader" in making noise.. |
@ivy Understood about the I can think about things a little more and figure out a way to get rid of the bug (because I think it's pretty egregious). |
From PR go-gomail#97