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.

Sign up for GitHub

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

Jump to bottom

Amended toService enabling the provision of a one-off httpConfig #1068

Closed
wants to merge 2 commits into from
Closed

Amended toService enabling the provision of a one-off httpConfig #1068

wants to merge 2 commits into from

Conversation

steinerj
Copy link

@steinerj steinerj commented Mar 2, 2015

This was a bug IMHO. All other restangular resources allow for providing a one-off httpConfig to be used with a subsequent server request. There is no way, to provide a httpConfig for POST-requests issued by objects created with toService. I don't see a reason why this shouldn't be allowed for .service created ones as well...

This problem was also brought up in Issue #420

Tests pass. I'm happy to add a unit test, but I'm not sure if that's the "final spec" for objects created by toService.

@steinerj
Amended toService enabling the provision of a one-off httpConfig
1c12b4a 
This was a bug IMHO. All other restangular resources allow for providing a one-off httpConfig to be provided for a subsequent server request. I don't see a reasons why this shouldn't be allowed for .service created ones as well...
@steinerj steinerj mentioned this pull request Mar 2, 2015
Closed
@steinerj
Copy link
Author

This PR is rather stale :) Could somebody at least comment on the issue?

@gustavohenke
Copy link

Could somebody at least comment on the issue?

I can!

I want this PR applied 👍

@madflow
Copy link
Contributor

madflow commented May 21, 2015

@mgonto Could you please merge this PR or comment on it? Thanks!

@daviesgeek
Copy link
Collaborator

Would you mind writing a test for this?

@daviesgeek daviesgeek self-assigned this Mar 25, 2016
@madflow
Copy link
Contributor

madflow commented Apr 14, 2016

Happy one year PR anniversary! I am not sure if @steinerj is still interested in this PR. However - I have an interest in getting this merged and I would volunteer if necessary. I have not contributed to restangular before, so I would need some guidance how to write a test. @steinerj asked for a "final spec" - @daviesgeek can you comment on this?

@daviesgeek
Copy link
Collaborator

@steinerj any comments?

@madflow as far as unit tests go, take a look at the current tests. It should be relatively self-explanatory, but if you have any questions, read the Jasmine docs. If you still have questions, feel free to let me know :)

madflow added a commit to madflow/restangular that referenced this pull request May 11, 2016
@madflow
Replace mgonto#1068
19e6f95
madflow added a commit to madflow/restangular that referenced this pull request May 11, 2016
@madflow
First test for mgonto#1068
0270e02
@daviesgeek
Copy link
Collaborator

@madflow Looks like you referenced this PR in your commit but didn't open a PR for your own branch?

madflow added a commit to madflow/restangular that referenced this pull request Jun 17, 2016
@madflow
Replace mgonto#1068
180d280
madflow added a commit to madflow/restangular that referenced this pull request Jun 17, 2016
@madflow
First test for mgonto#1068
e65c1b7
@madflow madflow mentioned this pull request Jun 17, 2016
Merged
2 tasks
@madflow
Copy link
Contributor

madflow commented Jun 17, 2016

I wrote a first test - not sure if this will be "enough". Since this PR is stale - I opened a new one #1392

This PR can safely be closed I believe.

@daviesgeek
Copy link
Collaborator

Thanks @madflow

Closing in favor of #1392

@daviesgeek daviesgeek closed this Jun 17, 2016
bostrom pushed a commit that referenced this pull request Nov 20, 2016
@madflow @bostrom
feat(service): add withHttpConfig to objects created with .service
e8f7295 
* Replacement for PR #1068 (#1392)

* First test for #1068

* Added doc
@steinerj
Copy link
Author

Sorry guys, haven't been paying attention. Happy @madflow's PR is done and dusted!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@daviesgeek daviesgeek

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@steinerj @gustavohenke @madflow @daviesgeek