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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/request response headers #1085

Closed
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@veryrusty
Member

veryrusty commented Dec 31, 2015

A dose of JFDI for #1039 馃槈

  • New response header keywords response_header, response_headers, push_response_header. Existing header keywords deprecated.
  • Adds new keyword for accessing request headers: request_header with simple test.
  • Updated docs

veryrusty added some commits Dec 30, 2015

Rename response header keywords
A request/response cycle involves two sets of headers. The existing
`header` keyword is ambiguous; is that request or response headers?

Add the prefix `response_` to the keywords; `header`, `headers` and
`push_header` to make it clear these manipulate response headers,
while deprecating the existing keywords.
Add `request_header` keyword
New keyword for accessing request headers, making those as easy to access
as response headers. Saves typing `request->headers->header(...)` :D
Ref #1039.
Test for `request` and `request_header` keywords
Updated the existing t/dsl.t test which tested the `request` keyword to
use Plack::Test->create(), then moved to the dsl subdir to add extra
subtest for new `reuest_header` keyword.
@xsawyerx

This comment has been minimized.

Member

xsawyerx commented Jan 6, 2016

I'm a bit worried about another deprecation notice close to when we're deprecating the old plugin interface. I don't want to overwhelm users. Can we comment those for now?

@SysPete

This comment has been minimized.

Member

SysPete commented Jan 6, 2016

馃憤 for delaying deprecation notices especially with the number we're already seeing with P2.

Comment deprecation warning on existing keywords
Timing is everything. With the reworked Plugin codebase due to be merged
soon, this is not the time be deprecating existing keywords.

Flag the keyword deprecation as a TODO after a period of stability.
@veryrusty

This comment has been minimized.

Member

veryrusty commented Jan 6, 2016

I concur that there are enough changes happening with the upcoming merge of Plugin2.

Deprecation warnings commented out and flagged as a TODO after a period of stability. 馃槃

@xsawyerx

This comment has been minimized.

Member

xsawyerx commented Mar 28, 2016

馃憤 from me.

@veryrusty 鉂わ笍
@mickeyn would like to have your comment on this, please.

@mickeyn

This comment has been minimized.

Contributor

mickeyn commented Mar 29, 2016

seems legit.

@veryrusty++

@cromedome

This comment has been minimized.

Contributor

cromedome commented Mar 31, 2016

馃憤 from me as well. Thanks @veryrusty!

@veryrusty

This comment has been minimized.

Member

veryrusty commented Apr 1, 2016

Rebased and merged as 431d8d8. Thanks everyone!

@veryrusty veryrusty closed this Apr 1, 2016

@veryrusty veryrusty deleted the feature/request_response_headers branch Apr 1, 2016

@xsawyerx

This comment has been minimized.

Member

xsawyerx commented Apr 2, 2016

xsawyerx added a commit that referenced this pull request Apr 19, 2016

v0.166001_01
    [ BUG FIXES ]
    * GH #1102: Handle multiple '..' in file path utilities.
      (Oleg A. Mamontov, Peter Mottram)
    * GH #1114: Fix missing prereqs as reported by CPANTS.
      (Mohammad S Anwar)
    * GH #1128: Shh warning if optional megasplat is not present.
      (David Precious)
    * GH #1139: Fix incorrect Content-Length header added by AutoPage
      handler (Michael Kr枚ll, Russell Jenkins)
    * GH #1144: Change tt tags to span in skel (Jason Lewis)
    * GH #1046: "no_server_tokens" configuration option doesn't work.
      (Sawyer X)
    # GH #1155, #1157: Fix megasplat value splitting when there are empty
      trailing path segments. (Tatsuhiko Miyagawa, Russell Jenkins)
      NOTE: Paths matching a megasplat that end with a '/' will now include
      an empty string as the last value. For the route pattern '/foo/**',
      the path '/foo/bar', the megasplat gives ['bar'], whereas '/foo/bar/'
      now gives ['bar','']. Joining the array of megasplat values will now
      always be the string matched against for the megasplit.

    [ DOCUMENTATION ]
    * GH #1119: Improve the deployment documentation. (Andrew Beverley)
    * GH #1123: Document import of utf8 pragma. (Victor Adam)
    * GH #1132: Fix spelling mistakes in POD (Gregor Herrmann)
    * GH #1134: Fix spelling errors detected by codespell (James McCoy)
    * GH #1153: Fix POD rendering error. (Sawyer X)

    [ ENHANCEMENTS ]
    * GH #1129: engine.logger.* hooks are called around logging a message.
      (Russell @veryrusty Jenkins)
    * GH #1146: Cleaner display of error context (Vernon Lyon)
    * GH #1085: Add consistent keywords for accessing headers;
      'request_header' for request, 'response_header', 'response_headers'
      and 'push_response_header' for response. (Russell @veryrusty Jenkins)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment