Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add an option to override SendGrid Client path version #850

Merged
merged 4 commits into from
Jul 20, 2020

Conversation

edwardrbaker
Copy link
Contributor

@edwardrbaker edwardrbaker commented Jun 6, 2019

Fixes

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the [Contribution Guide] and my PR follows them.
  • I updated my branch with the development branch.
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation about the functionality in the appropriate .md file
  • I have added in line documentation to the code I modified

Short description of what this PR does:

  • At my job, we have a facade API that sits in between consumers and SendGrid; it does some various things that are important to our business. We overrode the host which worked fine, but it also added /v3 to the end, so that our URLs ended up being http://company-email-api.domain.com/api/v1/email/v3.
  • I thought this was a simple update to give users more flexibility and should be fully backwards compatible.
  • I also updated a couple pieces of the Dockerfile; The URL for prism bash script was a 404, so I updated to the current existing install script. I also added support for dos2unix, which coverts line endings. My host machine is windows, and I had lots of issues getting the docker container to run for tests because of line endings.
  • I didn't apt-get remove dos2linux because I still needed to use it for the unit tests, manually.
  • I am new to PHP, Docker, PHPUnit, so if I've done something wrong, please let me know

If you have questions, please send an email to Twilio Sendgrid, or file a Github Issue in this repository.

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Jun 6, 2019
@edwardrbaker edwardrbaker marked this pull request as ready for review June 6, 2019 19:25
@thinkingserious thinkingserious added difficulty: medium fix is medium in difficulty type: community enhancement feature request not on Twilio's roadmap labels Jun 7, 2019
@thinkingserious
Copy link
Contributor

Thank you @erbaker!

I've added this to our backlog for a code review.

@childish-sambino childish-sambino removed the type: community enhancement feature request not on Twilio's roadmap label Jul 20, 2020
Copy link
Contributor

@childish-sambino childish-sambino left a comment

Choose a reason for hiding this comment

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

I'm renaming the option to be version as that's what it's called in the underlying client. Essentially, the part between the host name the start of the request path.

@childish-sambino childish-sambino changed the title Adding an option to override SendGrid Client path feat: Add an option to override SendGrid Client path version Jul 20, 2020
@childish-sambino childish-sambino merged commit 0599642 into sendgrid:master Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium fix is medium in difficulty status: code review request requesting a community code review or review from Twilio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants