Skip to content
This repository has been archived by the owner on Jan 20, 2024. It is now read-only.

first pass at implementing execute method, closes #48 #49

Merged
merged 3 commits into from
May 5, 2015

Conversation

nlf
Copy link
Contributor

@nlf nlf commented Mar 20, 2015

Also added grunt-cli to dev dependencies so I wouldn't have to globally install it.

@@ -20,6 +20,7 @@
"chai": "~1.9.1",
"grunt": "~0.4.5",
"grunt-bump": "~0.0.14",
"grunt-cli": "^0.1.13",
Copy link
Contributor

Choose a reason for hiding this comment

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

please change this to "~0.1.13"

@vernak2539
Copy link
Contributor

Please add test cases

},
Requests: {
Name: type,
Parameters: props
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this sounds kind of nitpicky, but why not params instead of props? It just seems to make better since.

Copy link

Choose a reason for hiding this comment

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

I believe @nlf used props because that's in keeping with the style of the rest of the module.

@vernak2539
Copy link
Contributor

@aydrian @nlf @gscott I see nothing wrong with this PR minus the missing test cases. Once they are added, I will publish a new version

@nlf
Copy link
Contributor Author

nlf commented May 5, 2015

added a test.

i had a sporadic issue with the last test in test/specs/function-soapRequest.js erroring with an ECONNRESET but it was super inconsistent, and happened to me before i added new tests so i didn't try to fix it.

vernak2539 added a commit that referenced this pull request May 5, 2015
first pass at implementing execute method, closes #48
@vernak2539 vernak2539 merged commit 6575a36 into salesforce-marketingcloud:master May 5, 2015
@vernak2539
Copy link
Contributor

published as v1.1.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants