You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently, the notification attachments section of the docs says the attachMany() method is available on the MailMessage class. This is wrong; that method is only available on Mailables.
At first I was going to submit a PR for the docs, but when I looked at the code the attach and attachData methods are very similar in both classes; the Mailable class does some additional uniqueness checks. It seems to me as though some (if not all) of the attachment functionality could easily be extracted to a trait and make things more consistent.
I don't want to start a PR unless I'm missing something obvious, and there's a good reason for keeping these separate?
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
Currently, the notification attachments section of the docs says the
attachMany()
method is available on theMailMessage
class. This is wrong; that method is only available onMailable
s.At first I was going to submit a PR for the docs, but when I looked at the code the
attach
andattachData
methods are very similar in both classes; the Mailable class does some additional uniqueness checks. It seems to me as though some (if not all) of the attachment functionality could easily be extracted to a trait and make things more consistent.I don't want to start a PR unless I'm missing something obvious, and there's a good reason for keeping these separate?
Beta Was this translation helpful? Give feedback.
All reactions