search

drewblas / aws-ses

Provides an easy ruby DSL & interface to AWS SES
MIT License
549 stars 108 forks source link

Several changes to the responses, fixed XML in email test, added address test & more #3

Closed sshaw closed 13 years ago

sshaw commented 13 years ago

Hi, currently the responses ignore the RequestMetadata (containing the RequestId). I added support for this. It required a slight modification to how the responses access the parsed metadata. As a result I added an address test case (but not an error response test -which is needed).

The XML in the email test was incorrect so I fixed it.

I feel the tests are a little hectic. I wanted to factor out some common ops into shoulda macros but I'm short on time this morning.

What are your feelings on creating attribs to access the response's fields? I did this for the email response's message id.

drewblas commented 13 years ago

I think attributes for the response fields are a good idea. It's one of the main reasons I have each response as a separate class. Just haven't gotten that far yet.

I'm going to merge this in hopefully today for a new release

drewblas commented 13 years ago

It looks like your last commit has a line-ending issue that I can't merge in. Any chance you do the pull request without rewriting the line endings of the project? Sorry!

sshaw commented 13 years ago

Try this one.

jfernandez commented 13 years ago

Any update on this drewblas? Message ID's are a must if you want to automate the bounced email logic and parsing.

drewblas commented 13 years ago

Hi sshaw,

I was not able to merge these changes cleanly, the line ending changes still caused major problems. However, I was able to extract your changes and add them where appropriate. Thank you very much. I've added you to the contributors list and released a new version of the gem with your changes.

I understand this may make it difficult for your branch to continue, as I think it has irrepairably diverged, so I'm very sorry for that.

I hope you'll continue to submit patches for any other functionality you need in the future. Drew

drewblas commented 13 years ago

Hi jfernandez,

This functionality is now available in version 0.4.0 of the gem. I'm sorry for the delay.

Drew

jfernandez commented 13 years ago

Awesome! thanks

sshaw commented 13 years ago

Yeah sorry about the trouble. My 3rd commit introduced the EOL change, but the 4th fixed this. I don't know that much about git, is it possible it will still merge the 3rd commit even though it's changes were superseded by the 4th commit?

Moving forward, I think an easy fix for me would be to delete my branch and fork from your latest.

-Skye