Skip to content

Conversation

@jonasschnelli
Copy link
Contributor

  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 extension like format chosing (like rest/tx/<hash>.json (as originally mentioned in Add unauthenticated HTTP REST interface #2844)

@laanwj laanwj added this to the 0.10.0 milestone Nov 27, 2014
@sipa
Copy link
Member

sipa commented Nov 27, 2014

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).

@laanwj
Copy link
Member

laanwj commented Nov 27, 2014

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.

@jonasschnelli
Copy link
Contributor Author

Agreed. It was a stupid idea to support uppercase/mixed-case URIs.
Just removed the toLowercase transition.
I also fixed the coding style (nit me if i didn't do it proper).

@laanwj
Copy link
Member

laanwj commented Nov 27, 2014

utACK

@jonasschnelli jonasschnelli force-pushed the rest-format-options branch 2 times, most recently from bbada20 to bea2170 Compare November 27, 2014 13:00
@jonasschnelli
Copy link
Contributor Author

Update coding style by auto.-reformatting with the src/.clang-format file.
Tell me if there are still uncommon formatting.

@laanwj
Copy link
Member

laanwj commented Nov 27, 2014

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`
@jgarzik
Copy link
Contributor

jgarzik commented Nov 27, 2014

Looks good, will test

@sipa
Copy link
Member

sipa commented Nov 28, 2014

Slightly tested ACK

@laanwj laanwj merged commit 8a5c951 into bitcoin:master Dec 2, 2014
laanwj added a commit that referenced this pull request Dec 2, 2014
8a5c951 [REST] make selection of output-format mandatory, support dot url syntax (Jonas Schnelli)
laanwj added a commit that referenced this pull request Dec 2, 2014
reddink pushed a commit to reddcoin-project/reddcoin-3.10 that referenced this pull request May 27, 2020
(cherry picked from commit 90f7aa7)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants