Skip to content

Conversation

eexit
Copy link
Contributor

@eexit eexit commented Mar 26, 2015

Hello,

  • Added IP pool support + test
  • Added comments
  • Normalized PHP files with PHP-FIG recommendations + set a coding style consistency
  • Minor improvements

Review will be easier when ignoring whitespace changes: https://github.com/sendgrid/smtpapi-php/pull/5/files?w=1

Would be really nice to have this merged asap.

Thank you very much,

Choose a reason for hiding this comment

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

Should this have $GIT_VERSION ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea, I didn't write this, I just changed the indentation :)

@mbernier
Copy link

Other than the question about $GIT_VERSION, the changes look legit.

Left to do:

  • Confirm $GIT_VERSION question
  • Test the changes

@eexit
Copy link
Contributor Author

eexit commented Mar 26, 2015

@mbernier I tested this PR in our system integration and it works perfectly. What else test would you like to be done?

Thanks for your review btw!

@mbernier
Copy link

We'll run it through our internal tests as soon as we can and then hopefully get it merged.

@eexit
Copy link
Contributor Author

eexit commented Mar 26, 2015

Oh, right! Great!

Thanks :)

eddiezane added a commit that referenced this pull request Mar 27, 2015
v0.3.0 — IP Pool support + minor improvements
@eddiezane eddiezane merged commit 3f1422a into sendgrid:master Mar 27, 2015
@eddiezane
Copy link
Contributor

Thanks so much for the PR!

What did you use to lint the codebase for FIG standards?

@eexit
Copy link
Contributor Author

eexit commented Mar 27, 2015

@eddiezane Thanks!
I quickly did it by hand... However, there are some PHP beautifuler online but I never tested yet :)

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.

3 participants