Skip to content
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

Added an option to manually set filename of attachment #56

Closed
wants to merge 1 commit into from

Conversation

slavikm
Copy link
Contributor

@slavikm slavikm commented Mar 30, 2016

If the name of the attachment is different the the actual filename on disk, we should have an easy option to change it

@slavikm
Copy link
Contributor Author

slavikm commented Mar 30, 2016

@alexcesaro sorry about the noise with the 2 PRs. I did not see that my teammate already created a PR on the exact same issue.

@@ -264,6 +264,14 @@ func SetHeader(h map[string][]string) FileSetting {
}
}

// SetFilename is a file setting to set the name of the attachment if the name is different
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please limit the line length to 80 characters and add a dot at the end of the sentence.

I think Rename() might be more clear. SetFilename can be confusing, some people could think you have to put the filename on disk here.

@alexcesaro
Copy link
Member

Thanks, I commented the code.

Please also merge all your commits (I only want one commit at the end) and add an example in example_test.go.

@slavikm
Copy link
Contributor Author

slavikm commented Mar 31, 2016

@alexcesaro Not sure how the examples are being used but see the last commit - squashed and added example.

@alexcesaro alexcesaro closed this in 4291610 Apr 1, 2016
@alexcesaro
Copy link
Member

Thank you! I slightly edited your commit to fix the example.

For your information, rules about naming examples can be found in the documentation of the testing package. And you can see the result in the Gomail documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants