SEP-24: Add lang field to GET /transactions and /transaction
#1191
Conversation
ecosystem/sep-0024.md
Outdated
| @@ -663,6 +663,7 @@ Name | Type | Description | |||
| `limit` | int | (optional) The response should contain at most `limit` transactions. | |||
| `kind` | string | (optional) The kind of transaction that is desired. Should be either `deposit` or `withdrawal`. | |||
| `paging_id` | string | (optional) The response should contain transactions starting prior to this ID (exclusive). | |||
| `lang` | string | (optional) Defaults to `en`. Language code specified using [ISO 639-1](https://en.wikipedia.org/wiki/ISO_639-1). | |||
There was a problem hiding this comment.
| `lang` | string | (optional) Defaults to `en`. Language code specified using [ISO 639-1](https://en.wikipedia.org/wiki/ISO_639-1). | |
| `lang` | string | (optional) Defaults to `en`. Language code specified using [RFC 4646](https://datatracker.ietf.org/doc/html/rfc4646) which means it can also accept locale in the format `en-US`. |
There was a problem hiding this comment.
@lijamie98 Thanks for opening this PR!
I tried pushing this new change to your branch but seems like I don't have permission. So I created this PR against your branch with a suggestion to update the lang definition in all places of Sep-24: #1192
There was a problem hiding this comment.
Thanks for pushing the PR to make all lang definition consistent. 👍 I had merged the #1192.
There was a problem hiding this comment.
@lijamie98 thanks Jamie, I think you also need to accept the 2 suggestions I've made to your original PR here
There was a problem hiding this comment.
Would you mind also adding the lang change to the Changelog at the bottom? Thanks!
There was a problem hiding this comment.
@lijamie98 sorry but I think the suggestion from this very comment has not been accepted(pushed) yet ☝️ (seems like only 1 of the suggestions has been accepted)
The PR change for line 666 is still using the old text: "lang | string | (optional) Defaults to en. Language code specified using ISO 639-1."
There was a problem hiding this comment.
it should read like this:
lang | string | (optional) Defaults to en. Language code specified using [RFC 4646] which means it can also accept locale in the format en-US.
There was a problem hiding this comment.
@lijamie98 line 666 is still not updated
There was a problem hiding this comment.
@CassioMG I will find out my issue about this git rebase which plays tricks on me. 😠
Updated and checked this time.
ecosystem/sep-0024.md
Outdated
| @@ -869,6 +870,7 @@ Name | Type | Description | |||
| `id` | string | (optional) The id of the transaction. | |||
| `stellar_transaction_id` | (optional) string | The stellar transaction id of the transaction. | |||
| `external_transaction_id` | (optional) string | The external transaction id of the transaction. | |||
| `lang` | string | (optional) Defaults to `en`. Language code specified using [ISO 639-1](https://en.wikipedia.org/wiki/ISO_639-1). | |||
There was a problem hiding this comment.
| `lang` | string | (optional) Defaults to `en`. Language code specified using [ISO 639-1](https://en.wikipedia.org/wiki/ISO_639-1). | |
| `lang` | string | (optional) Defaults to `en`. Language code specified using [RFC 4646](https://datatracker.ietf.org/doc/html/rfc4646) which means it can also accept locale in the format `en-US`. |
lijamie98
changed the title
lang field to GET /transactions and /transactionlang field to GET /transactions and /transaction
There was a problem hiding this comment.
Three asks:
-
Since this expands the capabilities of existing
langfields with the change in standard, can you bump the minor version to 2.3.0? -
Could you add a changelog entry?
-
And, could you make the RFC and ISO references footnote references? e.g. do this:
Language code specified using [RFC4646]. [RFC4646]: https://datatracker.ietf.org/doc/html/rfc4646
lijamie98
force-pushed
the
2022-05-05-add-lang
branch
4 times, most recently
from
b65eb28
to
6afb209
Compare
👍
Added.
@CassioMG had added the footnote references in each of the |
ecosystem/sep-0024.md
Outdated
| @@ -935,5 +937,6 @@ There is a small set of changes when upgrading from SEP-6 to SEP-24. | |||
| * Solar wallet: https://solarwallet.io | |||
|
|
|||
| ## Changelog | |||
| * `v2.3.0`: Add `lang` field to GET `/transactions` and `/transaction`. Make `lang` field definition consistent. ([#1191](https://github.com/stellar/stellar-protocol/pull/1191)) | |||
There was a problem hiding this comment.
Suggest change to lead with the larger change, and be explicit about what is changing:
| * `v2.3.0`: Add `lang` field to GET `/transactions` and `/transaction`. Make `lang` field definition consistent. ([#1191](https://github.com/stellar/stellar-protocol/pull/1191)) | |
| * `v2.3.0`: Change `lang` format from [ISO639-1] to [RFC4646] which is a superset of [ISO639-1]. Add `lang` field to GET `/transactions` and `/transaction`. ([#1191](https://github.com/stellar/stellar-protocol/pull/1191)) |
|
@lijamie98 I pushed a change in 840ba3e using footnotes. Feel free to revert the commit if you disagree. |
lijamie98
force-pushed
the
2022-05-05-add-lang
branch
2 times, most recently
from
ed64389
to
62accef
Compare
lijamie98
force-pushed
the
2022-05-05-add-lang
branch
from
62accef
to
27be536
Compare
LGTM! |
(cherry picked from commit 840ba3e)
Looks like the commit got accidentally clobbered in a force-push. Just re-pushed it as d536285. |
|
@leighmcculloch Thanks for merging the PR. |
No description provided.