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

Pass in req and res directly to PlumberStep$exec(req, res) #666

Merged
merged 19 commits into from
Sep 3, 2020
Merged

Pass in req and res directly to PlumberStep$exec(req, res) #666

merged 19 commits into from
Sep 3, 2020

Conversation

schloerke
Copy link
Collaborator

@schloerke schloerke commented Aug 31, 2020 •
edited
Loading

  • De-duplicates all matched formal arguments to avoid an error while processing the route
    • (No error is thrown anymore)
  • Allow for unnamed arguments to be passed through to endpoint

PR task list:

  • Update NEWS
  • Add tests
  • Update documentation with devtools::document()

@schloerke schloerke added this to the v1.0.0 - Next CRAN release milestone Aug 31, 2020
@schloerke schloerke requested a review from jcheng5 August 31, 2020 21:54
@schloerke
Add information into req$args incrementally.
3ad9f53 
This avoids circular dependencies when setting in filters and reading within the route.

This also matches historical behavior. So it's a welcome change.
@schloerke
fix pkgdown
8e5541c
cpsievert
cpsievert reviewed Sep 1, 2020
View reviewed changes
R/plumber-step.R Outdated Show resolved Hide resolved
cpsievert
cpsievert reviewed Sep 1, 2020
View reviewed changes
NEWS.md Outdated Show resolved Hide resolved
cpsievert
cpsievert reviewed Sep 1, 2020
View reviewed changes
R/plumber-step.R Outdated Show resolved Hide resolved
@cpsievert cpsievert self-requested a review September 1, 2020 19:31
cpsievert
cpsievert approved these changes Sep 1, 2020
View reviewed changes
Copy link
Contributor

@cpsievert cpsievert left a comment

Choose a reason for hiding this comment

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

LGTM pending suggestions

@schloerke
Copy link
Collaborator Author

@jcheng5 gentle ping

jcheng5
jcheng5 reviewed Sep 3, 2020
View reviewed changes
NEWS.md Outdated Show resolved Hide resolved
jcheng5
jcheng5 reviewed Sep 3, 2020
View reviewed changes
R/plumber-step.R Outdated Show resolved Hide resolved
jcheng5
jcheng5 reviewed Sep 3, 2020
View reviewed changes
R/plumber-step.R Outdated Show resolved Hide resolved
jcheng5
jcheng5 reviewed Sep 3, 2020
View reviewed changes
R/plumber-step.R Outdated Show resolved Hide resolved
jcheng5
jcheng5 reviewed Sep 3, 2020
View reviewed changes
R/plumber-step.R Outdated Show resolved Hide resolved
jcheng5
jcheng5 reviewed Sep 3, 2020
View reviewed changes
R/plumber-step.R Outdated Show resolved Hide resolved
jcheng5
jcheng5 reviewed Sep 3, 2020
View reviewed changes
R/plumber.R Outdated Show resolved Hide resolved
jcheng5
jcheng5 reviewed Sep 3, 2020
View reviewed changes
R/plumber.R Outdated Show resolved Hide resolved
@schloerke schloerke merged commit 820af6b into master Sep 3, 2020
@schloerke schloerke deleted the exec_req_res branch September 3, 2020 21:31
schloerke added a commit that referenced this pull request Sep 3, 2020
@schloerke
Merge branch 'master' into rc-v1.0.0
8b6c304 
* master:
  Pass in `req` and `res` directly to `PlumberStep$exec(req, res)` (#666)
  Make `postBody` a delayed assignment. Allows for removal of option. (#665)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcheng5 jcheng5 jcheng5 approved these changes

@cpsievert cpsievert cpsievert approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@schloerke @jcheng5 @cpsievert