Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add Api Version differences #2023

Closed
wants to merge 4 commits into from
Closed

Add Api Version differences #2023

wants to merge 4 commits into from

Conversation

PeterChen13579
Copy link
Contributor

Documenting issue 4545

@PeterChen13579
Copy link
Contributor Author

Hi @mDuo13! There was a bunch of PR's that got merged (here, here and here regarding bugs/discrepancies for certain user input calls. In general, all the fix(new behavior) is set for api_version 2, while the old behavior is set for api_version 1. I was wondering how we should document this in the docs? Take a look at my changes as an example. (doubt it will actually be like this though) Thanks for the help 😄

@intelliot
Copy link
Contributor

For simplicity, I suggest just documenting the latest version (and putting "api_version": 2 in the request). Of course, this should only be merged/deployed after the new version has been widely deployed in production. There should also an "added in" badge to show which version of rippled is being documented.

There can be a small note to indicate that older API versions exist, perhaps with a link to https://github.com/XRPLF/rippled/blob/develop/API-CHANGELOG.md for those who want to read about those historical versions.

@intelliot
Copy link
Contributor

"here" is not good anchor text. @thejohnfreeman or @mDuo13 , could you perhaps explain why, and offer an alternative?

thejohnfreeman
thejohnfreeman suggested changes Aug 9, 2023
View reviewed changes
Copy link
Contributor

@thejohnfreeman thejohnfreeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Links to "here" hide the destination and are hostile to screen readers. Prefer linking to nouns at the end of a sentence.

content/references/http-websocket-apis/public-api-methods/account-methods/account_info.md Outdated
## Api Version 2
* - Specifying invalid signer_lists value returns invalidParams error

Older Api versions can be used. Documented [here][api-version-history]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Older Api versions can be used. Documented [here][api-version-history]
Older [API versions][api-version-history] can be used.

content/references/http-websocket-apis/public-api-methods/account-methods/account_tx.md Outdated
* - Specifying `ledger_index_min`, `ledger_index_max` and `ledger_index` will result in an invalidParams error
* - Specifying `ledger_index_min` or `ledger_index_max` out of a valid range will result in lgrIdxMalformed.

Older Api versions can be used. Documented [here][api-version-history]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Older Api versions can be used. Documented [here][api-version-history]
Older [API versions][api-version-history] can be used.

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

Older Api versions can be used. Documented [here][api-version-history]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Older Api versions can be used. Documented [here][api-version-history]
Older [API versions][api-version-history] can be used.

@PeterChen13579
Copy link
Contributor Author

@intelliot @thejohnfreeman Thank you for the feedback, that makes complete sense! 😄 I changed it to what @thejohnfreeman suggested~

@thejohnfreeman
Copy link
Contributor

I think this will need a style review from one of our writers. My thoughts and opinions:

  • "API" needs to be fully capitalized everywhere it is used. Never "Api".
  • There should not be a section named "API Version 2". We should not be adding new sections for every API version. I anticipate, especially now that the gates are open, that we will be adding many versions. Alternatively, we can have a CHANGELOG section that lists the changes in each API version, or we can add notes to existing sections, e.g. "Possible Errors", tagged with "New in version 2" or "Added in version 2" or "Changed in version 2" or similar.
  • We already have a section on xrpl.org explaining API Versioning, and we already document how to choose the API version in a request. We should link the CHANGELOG on GitHub from that page, and we should link the API Versioning section from every "Added in version X" note in the rest of the documentation.

@PeterChen13579
Copy link
Contributor Author

Hi, just a heads up, my internship with Ripple will be ending next Friday. I don't know if there will be a style guideline by then for API version 2, so perhaps @mDuo13 can take over or assign my few changes to someone else. Thanks

@mDuo13
Copy link
Collaborator

mDuo13 commented Sep 7, 2023

I plan to take a different approach to document API v2 differences before it becomes enabled (currently expected in server version 1.13.0)

@mDuo13 mDuo13 closed this Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thejohnfreeman thejohnfreeman thejohnfreeman requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@PeterChen13579 @intelliot @thejohnfreeman @mDuo13