chore(docs): update definitions.json generation docs
#2947
Conversation
WalkthroughThis pull request updates documentation related to the process for updating the Changes
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
mvadari
requested review from
khancode
and removed request for
achowdhry-ripple
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/ripple-binary-codec/src/enums/README.md (1)
60-60: Updated Definitions Generation Link: The link now points to this tool, which is a clearer and more localized instruction for generating the definitions file. Ensure that the relative link remains valid when viewed from this README.CONTRIBUTING.md (1)
153-157: Minor Language Consistency: In the new “Updatingdefinitions.json” section, consider changing “afterwards” to “afterward” to align with American English usage.- if the output needs manual intervention afterwards, consider updating the script instead. + if the output needs manual intervention afterward, consider updating the script instead.🧰 Tools
🪛 LanguageTool
[locale-violation] ~155-~155: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...if the output needs manual intervention afterwards, consider updating the script instead. ...(AFTERWARDS_US)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CONTRIBUTING.md(1 hunks)packages/ripple-binary-codec/CONTRIBUTING.md(1 hunks)packages/ripple-binary-codec/src/enums/README.md(2 hunks)packages/ripple-binary-codec/src/enums/xrpl-definitions-base.ts(1 hunks)packages/ripple-binary-codec/src/enums/xrpl-definitions.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/ripple-binary-codec/src/enums/README.md (1)
Learnt from: mvadari
PR: XRPLF/xrpl.js#2826
File: packages/ripple-binary-codec/src/enums/definitions.json:33-57
Timestamp: 2025-03-24T09:13:29.089Z
Learning: When reviewing diffs in `definitions.json`, ensure to distinguish between entries that are duplicates and entries that are being moved or reordered to avoid false positives.
🪛 LanguageTool
CONTRIBUTING.md
[locale-violation] ~155-~155: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...if the output needs manual intervention afterwards, consider updating the script instead. ...
(AFTERWARDS_US)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: snippets (18.x)
🔇 Additional comments (5)
packages/ripple-binary-codec/src/enums/xrpl-definitions.ts (1)
17-18: Documentation Update – Local Tool Reference: The updated comment now directs users to generate a newdefinitions.jsonusing the local tool at
packages/ripple-binary-codec/tools/generateDefinitions.jsrather than an external URL. This update clearly reflects the new process.packages/ripple-binary-codec/src/enums/xrpl-definitions-base.ts (1)
41-43: Documentation Update – Relative Path Validation: The comments now instruct users to use the local tool at
packages/ripple-binary-codec/tools/generateDefinitions.jsfor generating a new definitions file. Please verify that the relative path works for all contributors.packages/ripple-binary-codec/src/enums/README.md (1)
72-73: Enhanced Contributor Instruction: The added instruction that instructs users to "Generate your owndefinitions.jsonfile from rippled source code using this tool" improves clarity. This update is consistent with other documentation changes.packages/ripple-binary-codec/CONTRIBUTING.md (1)
21-27: Updateddefinitions.jsonInstructions: The new section for updatingdefinitions.jsoninstructs contributors to use the script here. Ensure that the relative path is correct relative to this CONTRIBUTING file for a smooth contributor experience.CONTRIBUTING.md (1)
153-160: Clear and Updated Process Instructions: The updated steps provide clear guidance on generating and verifying thedefinitions.jsonfile using
./packages/ripple-binary-codec/tools/generateDefinitions.js. These instructions remove outdated references and streamline the contributor workflow.🧰 Tools
🪛 LanguageTool
[locale-violation] ~155-~155: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...if the output needs manual intervention afterwards, consider updating the script instead. ...(AFTERWARDS_US)
Co-authored-by: Raj Patel <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CONTRIBUTING.md (1)
153-160: Documentation Clarity and Numbering Consistency
- The new "Updating
definitions.json" section is much clearer in directing contributors to use the local generation script.- Consider updating "afterwards" to "afterward" to conform with American English style conventions.
- The step numbering appears inconsistent (steps "1.", "2.", then "5."). It might be clearer to renumber these sequentially (e.g., "1.", "2.", "3.") for improved readability.
- Optionally, you could add a brief clarification on when manual intervention is expected, which will help contributors decide if they need to update the script further.
🧰 Tools
🪛 LanguageTool
[locale-violation] ~155-~155: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...if the output needs manual intervention afterwards, consider updating the script instead. ...(AFTERWARDS_US)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CONTRIBUTING.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[locale-violation] ~155-~155: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...if the output needs manual intervention afterwards, consider updating the script instead. ...
(AFTERWARDS_US)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: browser (18.x)
- GitHub Check: snippets (20.x)
- GitHub Check: snippets (22.x)
- GitHub Check: integration (22.x)
- GitHub Check: snippets (18.x)
- GitHub Check: integration (18.x)
- GitHub Check: integration (20.x)
fix numbering
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CONTRIBUTING.md (1)
155-155: Use American English Variant.
Please replace "afterwards" with "afterward" for consistency with American English style.-... if the output needs manual intervention afterwards, consider updating the script instead. +... if the output needs manual intervention afterward, consider updating the script instead.🧰 Tools
🪛 LanguageTool
[locale-violation] ~155-~155: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...if the output needs manual intervention afterwards, consider updating the script instead. ...(AFTERWARDS_US)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CONTRIBUTING.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[locale-violation] ~155-~155: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...if the output needs manual intervention afterwards, consider updating the script instead. ...
(AFTERWARDS_US)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: integration (22.x)
- GitHub Check: snippets (22.x)
- GitHub Check: snippets (20.x)
- GitHub Check: snippets (18.x)
- GitHub Check: integration (18.x)
- GitHub Check: browser (18.x)
- GitHub Check: integration (20.x)
🔇 Additional comments (1)
CONTRIBUTING.md (1)
153-159: Clarify and Validate the Updated Steps for Generatingdefinitions.json.
The new section "Updatingdefinitions.json" clearly documents that contributors should use the script located at./packages/ripple-binary-codec/tools/generateDefinitions.js. The steps (pulling the latestrippledfrom thedevelopbranch, running the script, and verifying the output) are concise and align well with the recent process updates from PR #2826. Please verify that the relative path and assumptions (i.e. running the command from the repository root) hold true for most contributor environments.🧰 Tools
🪛 LanguageTool
[locale-violation] ~155-~155: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...if the output needs manual intervention afterwards, consider updating the script instead. ...(AFTERWARDS_US)
|
|
||
| Use [this repo](https://github.com/RichardAH/xrpl-codec-gen) to generate a new `definitions.json` file from the rippled source code. Instructions are available in that README. | ||
| 1. Clone / pull the latest changes from [rippled](https://github.com/XRPLF/rippled) - Specifically the `develop` branch is usually the right one. | ||
| 2. Run `node packages/ripple-binary-codec/tools/generateDefinitions.js path/to/rippled` (assuming you're calling this file from the root directory of xrpl.js). |
There was a problem hiding this comment.
| 2. Run `node packages/ripple-binary-codec/tools/generateDefinitions.js path/to/rippled` (assuming you're calling this file from the root directory of xrpl.js). | |
| 2. Run `npm run generateDefinitions path/to/rippled` (assuming you're calling this file from the root directory of xrpl.js). |
There was a problem hiding this comment.
As we discussed previously, that doesn't work.
There was a problem hiding this comment.
The npm script works for me. You need to be inside the packages/ripple-binary-codec directory.
There was a problem hiding this comment.
That makes the command more complex. Most people aren't going to be in that directory when executing this script, they're going to be in the root directory for xrpl.js. So then the command would have to be:
cd packages/ripple-binary-codec
npm run generateDefinitions path/to/rippled
# and now you're in a different directory and need to cd back to the root directory to do other thingsOr
(cd packages/ripple-binary-codec && npm run generateDefinitions path/to/rippled)Both of those aren't the most user-friendly commands, which is why I included what I did.
High Level Overview of Change
This PR adds documentation for how to generate the
definitions.jsonoutput via the new script introduced in #2826, and removes mention of the old script.Context of Change
#2826 didn't update contributing docs to tell contributors to use the new flow.
Type of Change
Did you update HISTORY.md?