-
Notifications
You must be signed in to change notification settings - Fork 38.7k
[REST] make selection of output-format mandatory, support dot url syntax #5376
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
Conversation
fdad684 to
ff05f57
Compare
|
Concept ACK, but I wouldn't do the case insensitivity. That just means more combinations to support in the future. Also, coding style (spaces after if etc). |
|
Agree with sipa. Please don't leave any wiggling room in the REST API. Aliasing is generally bad, for example with regard to webcaches and such. One URL should map to one query. |
ff05f57 to
a97233a
Compare
|
Agreed. It was a stupid idea to support uppercase/mixed-case URIs. |
|
utACK |
bbada20 to
bea2170
Compare
|
Update coding style by auto.-reformatting with the src/.clang-format file. |
bea2170 to
50baa4f
Compare
|
Looks good to me now. |
1. Remove the default format (binary) because `rest/block/<hash>/Hex` would end up delivering binary data. 2. List available formats when chosen format was not found (reduces need for documentation) 3. Change url syntax to dot extension like format chosing (like `rest/tx/<hash>.json`
50baa4f to
8a5c951
Compare
|
Looks good, will test |
|
Slightly tested ACK |
8a5c951 [REST] make selection of output-format mandatory, support dot url syntax (Jonas Schnelli)
(cherry picked from commit 90f7aa7)
rest/block/<hash>/Hexwould end up delivering binary data.rest/tx/<hash>.json(as originally mentioned in Add unauthenticated HTTP REST interface #2844)