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

Ensure that requestData has the correct method and type #231

Merged
merged 2 commits into from
Jan 24, 2017
Merged

Ensure that requestData has the correct method and type #231

merged 2 commits into from
Jan 24, 2017

Conversation

alexlafroscia
Copy link
Collaborator

No description provided.

@alexlafroscia
Copy link
Collaborator Author

It's pretty silly that we use this requestData object at all, IMO. We only use it to create the promise label, which we could just do from original hash argument. handleResponse could be called with the original XHR, which would eliminate the need for passing in the status text directly as well. Definitely a breaking change -- worth cleaning up for 3.0.0?

cc: @jherdman @taras

@coveralls
Copy link

Coverage Status

Coverage increased (+0.009%) to 97.024% when pulling f5b72cd on fix-issue-229 into df2f838 on master.

jherdman
jherdman approved these changes Jan 23, 2017
View reviewed changes
Copy link
Collaborator

@jherdman jherdman left a comment

Choose a reason for hiding this comment

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

LGTM. Given that 3.0 is going to be invasive, we may as well clean this up as well.

@alexlafroscia
Copy link
Collaborator Author

Hmm, so now there's a question of how to get this out into a release. master already has breaking changes.... Maybe we can branch from the last release, cherry-pick this and release again?

@alexlafroscia alexlafroscia merged commit a55f328 into master Jan 24, 2017
alexlafroscia added a commit that referenced this pull request Jan 24, 2017
@alexlafroscia
Ensure that requestData has the correct method and type (#231)
5d98a44 
* Install testdouble and testdouble-chai

* Ensure that `requestData` has the correct method and type

Closes #229

# Conflicts:
#	bower.json
#	package.json
#	yarn.lock
@locks locks deleted the fix-issue-229 branch July 17, 2017 12:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jherdman jherdman jherdman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alexlafroscia @coveralls @jherdman