drewr / postal

Clojure email support
MIT License
587 stars 85 forks source link

Reflection warnings #72

Closed laczoka closed 8 years ago

laczoka commented 8 years ago

Reflection warning, postal/message.clj:51:28 - reference to field getAddress can't be resolved. Reflection warning, postal/message.clj:52:28 - reference to field getPersonal can't be resolved. Reflection warning, postal/message.clj:139:5 - call to method setText on javax.mail.Message can't be resolved (no such method). Reflection warning, postal/message.clj:161:17 - call to postal.message.proxy$javax.mail.internet.MimeMessage$ff19274a ctor can't be resolved. Reflection warning, postal/message.clj:163:21 - call to method setHeader can't be resolved (target class is unknown). Reflection warning, postal/sendmail.clj:74:16 - call to java.lang.ProcessBuilder ctor can't be resolved.

charles-dyfis-net commented 8 years ago

Is any of this code you're calling with enough frequency for the reflection overhead to be measurable, or is this filed with the intent of clearing warnings for its own sake? I'd tend to expect the actual process of sending a message over the network, or invoking an external sendmail executable, to overwhelm any overhead added here.

sfnelson commented 8 years ago

Reflection warnings are not just about performance. For code like postal that interacts with Java APIs they can indicate actual errors, especially where the upstream API has changed and postal is no longer compatible.

Most importantly however, postal is a library that will be used by other people. Some of those people (like my team) will turn on reflection warnings to help identify problems in non-postal code. Postal's reflection warnings are just noise that gets in the way of seeing the messages that we actually want to see.

Please accept pull requests that resolve ambiguities and eliminate reflection warnings, and consider enabling reflection warnings in the postal leiningen configuration so that you don't cause noise for downstream users.

drewr commented 8 years ago

Thanks all. This has been addressed in #79.