search

mendicant-original / newman

Newman: a microframework for building email-based applications.
116 stars 11 forks source link

Bounce handling #9

Closed ericgj closed 12 years ago

ericgj commented 12 years ago

Pull request for issue #6, with tests. See what you think.

Note I rebased against upstream before pushing these changes, so really it's just the last commit of these 4 -- the other 3 were already merged in. Let me know if I should have done this differently.

practicingruby commented 12 years ago

While the functionality of this looks useful, I don't really know that I want to drag in a bunch of complicated bounce-handling heuristics from another project. Perhaps this is a good tool to be built as a Newman extension instead of part of core, at least at first (i.e. newman-bounces)? That'd give us a good chance to figure out how to flesh out a developer API for Newman.

practicingruby commented 12 years ago

btw, the reason I closed this is because the bounce_email.rb file looks like a real maintenance liability. It has commented out sections and "works for me" comments which make me worry about whether it will work for us :)

practicingruby commented 12 years ago

@ericgj: Out of curiosity, I looked into this a bit closer. This bounce handling makes it so that even including the word vacation in a subject field matches as a bounce. That seems way too aggressive. For example, the following test will fail.


  it "responds to regular email" do
    mailer.deliver_message(:to => 'test+bounce@test.com',
                           :subject => 'have a nice vacation')
    server.tick(@app)
    mailer.messages.first.subject.must_equal('not handled!')
  end

Also... the way I'd prefer patches (I think), is if you simply keep your master branch clean and pull upstream into it regularly. Then branch off of your clean master. That should prevent commits from getting repeated in your pull requests, I think