Skip to content

Conversation

beardyman
Copy link
Contributor

Major rework including removing dependencies on Guzzle and updating to use an instance based approach instead of a static one. Closes #12 and #20.

* The new resource is attached to this object as well as returned
* @return SparkPost\APIResource - the unwrapped resource
*/
public function setupUnwrapped (string $endpoint) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jasonrhodes: For what its worth, I don't really like the name of this function. Especially since its an externally facing function, I think we should change it but I don't really know what to call it.

@@ -22,25 +22,53 @@ After installing, you need to require Composer's autoloader:
require 'vendor/autoload.php';
```

## Setting up a Request Adapter
Copy link
Contributor

Choose a reason for hiding this comment

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

I would put an example composer.json file here too so people can just grab it if they want to use these deps, ie

{
  "require": {
    "sparkpost/php-sparkpost": "~1.0",
    "egeloen/http-adapter": "0.8.0",
    "guzzlehttp/guzzle": "6.1.0",
    "php-http/guzzle6-adapter": "0.1.0"
  }
}

@jasonrhodes
Copy link
Contributor

For all of the "triple backtick" code blocks, if you use "triple backtick" + php like '''php but with backticks, it'll use syntax highlighting which will help with readability too. You might have to include the opening <?php though.

@beardyman beardyman self-assigned this Oct 2, 2015
@richleland richleland added this to the v0.2.0 milestone Oct 2, 2015
@beardyman
Copy link
Contributor Author

ok this time I promise all of the \t's are gone

'from'=>'From Envelope <from@sparkpostbox.com>',
'html'=>'<html><body><h1>Congratulations, {{name}}!</h1><p>You just sent your very first mailing!</p></body></html>',
'text'=>'Congratulations, {{name}}!! You just sent your very first mailing!',
'substitutionData'=>['name'=>'YOUR FIRST NAME']
Copy link
Contributor

Choose a reason for hiding this comment

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

Still missing trailing comma here

@@ -2,6 +2,7 @@
"name": "sparkpost/php-sparkpost",
"description": "SDK for interfacing with SparkPost APIs",
"license": "Apache 2.0",
"version": "0.2.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is going to be 1.0 right?

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'm not sure, I want to talk to @richleland and @aydrian about that. I personally think it should be just because this breaks everything existing. But since its still a 0.* I'm not sure, we may be ok with that.

Copy link
Contributor

Choose a reason for hiding this comment

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

0.* versions have all kinds of problems, the sooner we get to 1.x the better imo, for everything (npm init defaults to 1.0.0 now because of how volatile and weird the semver rules are around 0.x versions)

…coverage filters in phpunit.xml to ignore test files because a utility function wasn't being used
@beardyman beardyman modified the milestones: v1.0.0, v0.2.0 Oct 7, 2015
@jasonrhodes
Copy link
Contributor

OK tested this locally, still works for sending transmissions, I say :shipit:

beardyman added a commit that referenced this pull request Oct 10, 2015
@beardyman beardyman merged commit 7c39c1e into master Oct 10, 2015
@beardyman beardyman deleted the http-adapter-update branch October 10, 2015 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants