-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Server field in HTTP API response header #2141
Conversation
a3b0c46
to
5965f66
Compare
Tests are passing. One of Travis builds randomly hanged. |
@@ -24,6 +24,7 @@ test_ls_cmd() { | |||
printf "Access-Control-Allow-Headers: X-Stream-Output, X-Chunked-Output\r\n" >>expected_output && | |||
printf "Access-Control-Expose-Headers: X-Stream-Output, X-Chunked-Output\r\n" >>expected_output && | |||
printf "Content-Type: text/plain\r\n" >>expected_output && | |||
printf "Server: go-ipfs/%s\r\n" $(ipfs version | cut -d" " -f 3) >>expected_output && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use ipfs version -n
instead of cut
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use ipfs version -n instead of cut
Just to make very sure we don't run into any superweird portability issues
8035a3a
to
f44eaf3
Compare
I've addressed your comments. Tests are passing. |
👍 |
9e74a0c
to
1d6e577
Compare
@@ -24,6 +24,7 @@ test_ls_cmd() { | |||
printf "Access-Control-Allow-Headers: X-Stream-Output, X-Chunked-Output\r\n" >>expected_output && | |||
printf "Access-Control-Expose-Headers: X-Stream-Output, X-Chunked-Output\r\n" >>expected_output && | |||
printf "Content-Type: text/plain\r\n" >>expected_output && | |||
printf "Server: go-ipfs/%s\r\n" $(ipfs version -n) >>expected_output && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delayed feedback -- it looks like circle-ci is failing because of the ordering of the expected headers -- now that server: is being set first, it needs to be expected first
👍 ❤️ apart from that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is weird as Travis was successful.
Resolves ipfs#625 Included in tests. License: MIT Signed-off-by: Jakub (Kubuxu) Sztandera <kubuxu@gmail.com>
This time both CircleCI and Travis went through. I've changed nothing. |
Yep LGTM now, thanks! |
Add Server field in HTTP API response header
LGTM Thanks |
Resolves #625