Skip to content

added verbs for put, patch and delete - #45

Merged
elorest merged 2 commits into
masterfrom
is/fix_other_verbs
May 10, 2017
Merged

elorest merged 2 commits into
masterfrom
is/fix_other_verbs

Conversation

@elorest

@elorest elorest commented May 9, 2017

Copy link
Copy Markdown
Member

Description of the Change

Adds verbs for put, patch and delete. Fairly small change that doesn't seem to break anything. Don't merge it yet but I wanted to post it so that you didn't waste time in the same direction tomorrow.

Why Should This Be In Core?

Routing all restful routes is useful for web frameworks.

Benefits

All verbs now work. Params are parsed before the route is looked up. Also the looked up node is saved and used since it was always added to the chain by default. This means we don't have to look up the same route twice per request.

Possible Drawbacks

Params have to always be parsed in order to route.

Applicable Issues

If you include params in your pipeline now it will cause an issue because it's being used by default.

@eliasjpr

eliasjpr commented May 9, 2017

Copy link
Copy Markdown
Contributor

I am ok with merging this now as long we agree to revisit this implementation

@elorest
elorest merged commit 6f3af07 into master May 10, 2017
@eliasjpr eliasjpr mentioned this pull request May 20, 2017
@eliasjpr
eliasjpr deleted the is/fix_other_verbs branch May 20, 2017 01:01
elorest pushed a commit that referenced this pull request Aug 26, 2017
elorest added a commit that referenced this pull request Aug 26, 2017
* fixed DockerFile and cleaned up flash in controller template

* fixed ecr rendering and bumped

* very happy with the progress even if it is 2AM. :(

* provision and deploy work so well :)

* added spinner so people won't think it's frozen
marksiemers pushed a commit to marksiemers/amber that referenced this pull request Oct 28, 2017
marksiemers pushed a commit to marksiemers/amber that referenced this pull request Oct 28, 2017
elorest added a commit that referenced this pull request Nov 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants