ledger method: update for 1.12.0 #2248

mDuo13 · 2023-11-01T23:18:07Z

Fix #2158

Update for 1.12.0 removing some deprecated fields from the response.
- Update example API responses and use includes for the example responses.
Mark the full and accounts parameters and the corresponding accountState response field as deprecated.
Fix an outdated note about close_flags being omitted from the JSON representation of a ledger header.
Clean up a few minor formatting inconsistencies, such as the non-alphabetical order of fields in the ledger header page, missing periods at end of description rows, and change "local port" (confusing term) to "localhost" (more clear).

github-actions · 2023-11-01T23:35:08Z

Link check report. 554061 links checked.
Success! No broken links found.

Preview: https://XRPLF.github.io/xrpl-dev-portal/pr-preview/ledger_remove_deprecated_fields/

Style Report

ckeshava

Hello @mDuo13 , thanks for this PR. I have asked a few clarifying questions.

I couldn't observe any changes in the behavior of close_flags. Although the description of the PR mentions it, I didn't see any changes. Did I miss something?

ckeshava · 2023-11-02T18:24:35Z

content/references/http-websocket-apis/public-api-methods/ledger-methods/ledger.md

@@ -191,27 +107,28 @@ The response follows the [standard format][], with a successful result containin

 | `Field`                        | Type    | Description                       |
 |:-------------------------------|:--------|:----------------------------------|
-| `ledger`                       | Object  | The complete header data of this ledger. |
-| `ledger.account_hash`          | String  | Hash of all account state information in this ledger, as hex |
-| `ledger.accountState`          | Array   | (Omitted unless requested) All the [account-state information](ledger-data-formats.html) in this ledger. |


I was of the understanding that we mark a field as [deprecated] and gracefully remove it from the responses. Have we informed the community sufficiently in advance?
My apologies if this was communicated before, I'm not aware of this development.

This is the communication. The code will still work without changes, but this updates the docs to communicate the deprecation.

oh okay :+1

ckeshava · 2023-11-02T18:27:04Z

content/references/http-websocket-apis/public-api-methods/ledger-methods/ledger.md

-| `queue_data`                   | Array   | _(Omitted unless requested with the `queue` parameter)_ Array of objects describing queued transactions, in the same order as the queue. If the request specified `expand` as true, members contain full representations of the transactions, in either JSON or binary depending on whether the request specified `binary` as true. Added by the [FeeEscalation amendment][]. [New in: rippled 0.70.0][] |
+| `queue_data`                   | Array   | _(Omitted unless requested with the `queue` parameter)_ Array of objects describing queued transactions, in the same order as the queue. If the request specified `expand` as true, members contain full representations of the transactions, in either JSON or binary depending on whether the request specified `binary` as true. |
+
+The `ledger.accountState` field (omitted unless requested with `"full": true` or `"accounts": true`) is deprecated.


Suggested change

The `ledger.accountState` field (omitted unless requested with `"full": true` or `"accounts": true`) is deprecated.

The `ledger.accountState` field is deprecated.

^^ I feel this would convey the meaning better, since full and accounts have been deprecated themselves

I felt the note was needed because otherwise it might be confusing to be told that a field you don't see is deprecated—people might wonder if it has been removed already. This makes it clear when you should expect to see the field even though it is deprecated.

ckeshava · 2023-11-02T18:29:46Z

content/references/http-websocket-apis/public-api-methods/ledger-methods/ledger.md

@@ -238,7 +155,7 @@ If the request specified `"owner_funds": true` and expanded transactions, the re
 * Any of the [universal error types][].
 * `invalidParams` - One or more fields are specified incorrectly, or one or more required fields are missing.
 * `lgrNotFound` - The ledger specified by the `ledger_hash` or `ledger_index` does not exist, or it does exist but the server does not have it.
-* `noPermission` - If you specified `full` or `accounts` as true, but are not connected to the server as an admin (usually, admin requires connecting on a local port).
+* `noPermission` - If you specified `full` or `accounts` as true, but are not connected to the server as an admin (usually, admin requires connecting on localhost).


Isn't full and accounts deprecated? Is it possible to get noPermission error even without these fields?

That's true, but deprecated does not mean removed, so it's still a real error that can occur.

ckeshava · 2023-11-02T18:36:06Z

dactyl-config.yml

@@ -3144,6 +3144,7 @@ pages:
            - en

    -   md: "@i18n/ja/references/http-websocket-apis/public-api-methods/ledger-methods/ledger.md"
+        outdated_translation: true


could you explain what this addition means?

It displays a warning on the Japanese version of the page telling users that the translated version of the page isn't up-to-date with the English version.

ckeshava

this looks good to me 👍

amarantha-k

lgtm

ledger method: update for 1.12.0

d470a0b

mDuo13 added this to the rippled 1.12.0 milestone Nov 1, 2023

mDuo13 added the content updates Updating, cleaning up, or rewriting existing content for better usability or new contexts label Nov 1, 2023

mDuo13 added the needs japanese translation PRs with English changes that need to be added/updated in the Japanese translation of the site label Nov 2, 2023

ckeshava reviewed Nov 2, 2023

View reviewed changes

ckeshava approved these changes Nov 2, 2023

View reviewed changes

amarantha-k approved these changes Nov 6, 2023

View reviewed changes

mDuo13 merged commit 443b48f into master Nov 13, 2023

mDuo13 deleted the ledger_remove_deprecated_fields branch November 13, 2023 21:28

tequdev removed the needs japanese translation PRs with English changes that need to be added/updated in the Japanese translation of the site label Dec 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ledger method: update for 1.12.0 #2248

ledger method: update for 1.12.0 #2248

mDuo13 commented Nov 1, 2023

github-actions bot commented Nov 1, 2023

ckeshava left a comment

ckeshava Nov 2, 2023

mDuo13 Nov 2, 2023

ckeshava Nov 2, 2023

ckeshava Nov 2, 2023

mDuo13 Nov 2, 2023

ckeshava Nov 2, 2023

mDuo13 Nov 2, 2023

ckeshava Nov 2, 2023

mDuo13 Nov 2, 2023

ckeshava Nov 2, 2023

ckeshava left a comment

amarantha-k left a comment

	The `ledger.accountState` field (omitted unless requested with `"full": true` or `"accounts": true`) is deprecated.
	The `ledger.accountState` field is deprecated.

ledger method: update for 1.12.0 #2248

ledger method: update for 1.12.0 #2248

Conversation

mDuo13 commented Nov 1, 2023

github-actions bot commented Nov 1, 2023

ckeshava left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ckeshava left a comment

Choose a reason for hiding this comment

amarantha-k left a comment

Choose a reason for hiding this comment