search

php-http / message

HTTP Message related tools
http://php-http.org
MIT License
1.29k stars 41 forks source link

adjust request matcher to the symfony model #36

Closed dbu closed 8 years ago

dbu commented 8 years ago

fix #33

this matcher has less options than the Symfony request matcher. a Symfony request is more like the ServerRequestInterface of psr 7 and we mainly work with client requests, so matching on the client ip makes no sense. and psr 7 requests have no attributes so i dropped that as well.

sagikazarmark commented 8 years ago

Isn't this a BC break? I think that message has already been tagged by @joelwurtz

dbu commented 8 years ago

code using the matcher would still "work" but the meaning of the first parameter has changed. it used to match anything of the uri: scheme, host or path. now it only applies to path. not sure if that could be a 1.2 version or is really too bad a BC break.

not sure how we should handle this. i don't want to add a separate class that does the same but different. should we delay merging this until we bump to 2.0?

sagikazarmark commented 8 years ago

Well, we either add a separate one which does the same and deprecates the other (and gets removed in 2.0) or just do this and break BC in 1.2. That would require a big fat warning.

From a BC point of view the first one seems to be better to me. Although I am not sure how many people might already use this feature (but this mentality is not the best for versioning).

dbu commented 8 years ago

okay, you are right. we do semver, we should not randomly decide to violate BC.

deprecated and add a new one as RequestMatcher. what do you think of this?

sagikazarmark commented 8 years ago

Sounds like a plan