Github user asfgit closed the pull request at:
https://github.com/apache/trafficserver/pull/455
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user shukitchan commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-185373900
thanks... let me merge it tonight.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your proj
Github user gtenev commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-185330773
- Reviewed and tested the patch and it looks OK.
- Verified that it does not break backward compatibility, tested end-to-end
against S3 with both virtual-hosted
Github user robguima commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-181994021
@zwoop @shukitchan please let me know if the std::string free version above
is OK - if so I will squash it thx
---
If your project is set up for it, you can rep
Github user robguima commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-181605333
@zwoop thought that was a pretty good argument myself. :) not a prob will
change it - also forgot you guys are multiplat...
---
If your project is set up for it
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-181604662
Yeah, the problem with it (and pretty much all of STL) is that it has
hidden (and sometimes) unpredictable memory allocations behavior. I did some
tests recently wi
Github user robguima commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-181600678
2) hmm string is one of the few things I like about C++ :) especially for,
like you said, code that is not in the main code path (e.g. debugging). It's
just conv
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-181594755
Couple of things on that last patch:
1) You have to run clang-format over the code, I'm fairly certain we would
never indent like this:
if (con_md
Github user robguima commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-181591169
@zwoop I will squash it to one commit if this change is OK.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHu
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-180911692
Changing to TSDebug is fine, I don't recall why printf() was used, probably
laziness :) I'll try to test this in our setup next week, but I agree that the
changes o
Github user robguima commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-180907419
Yes, we can be fairly sure it doesn't (break anything), because:
a) we've added functionality not changing the way it worked before (bug
aside). So, if conten
Github user shukitchan commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-180906411
I think we should use TSDebug as well.
The change should be "backwards compatible".
---
If your project is set up for it, you can reply to this emai
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-180905142
One question: Can we be sure this doesn't break anything "backwards
compatible" ? I.e. someone using the plugin today, it should just work the same
with these chang
Github user robguima commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-180702368
@zwoop done thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-180669497
Please squash this down to one commit IMO.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your p
Github user robguima commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-180640259
@shukitchan just to undo my formatting change from older patch - think
someone did that between 5.3 and 6.x
---
If your project is set up for it, you can reply
Github user shukitchan commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-180587604
looks good now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have t
Github user shukitchan commented on the pull request:
https://github.com/apache/trafficserver/pull/455#issuecomment-180535806
@jpeach , can you take a look as well?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user shukitchan commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/455#discussion_r52065719
--- Diff: plugins/experimental/s3_auth/s3_auth.cc ---
@@ -346,26 +349,48 @@ S3Request::authorize(S3Config *s3)
// If the configuration is a "v
GitHub user robguima opened a pull request:
https://github.com/apache/trafficserver/pull/455
TS-4176: s3_auth add support for matrix params
@shukitchan please review
https://issues.apache.org/jira/browse/TS-4176
You can merge this pull request into a Git repository by runn
20 matches
Mail list logo
