Skip to content

adds copilot review instructions#489

Open
Tapanito wants to merge 2 commits intomasterfrom
tapanito/copilot
Open

adds copilot review instructions#489
Tapanito wants to merge 2 commits intomasterfrom
tapanito/copilot

Conversation

@Tapanito
Copy link
Copy Markdown
Collaborator

@Tapanito Tapanito commented Mar 4, 2026

High Level Overview of Change

Context of Change

Type of Change

  • New XLS Draft
  • XLS Update (changes to an existing XLS)
  • XLS Status Change (e.g., Draft → Final, Draft → Stagnant)
  • Process/Meta (changes to CONTRIBUTING.md, XLS-1, templates, etc.)
  • Infrastructure (CI, workflows, scripts, website)
  • Documentation (README updates, typo fixes)

## XLS_TEMPLATE.md — Required Top-Level Sections

```
## 1. Abstract ← required
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm worried about this being too rigid about numbers

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Right, that it would complain if we introduced a section that is not in the list, and break the numbering?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a GitHub Copilot instruction document under .github/ to capture XRPL-Standards repo structure and key validation/template rules for reviewing and authoring XLS Markdown specs.

Changes:

  • Add .github/copilot-instructions.md documenting repo layout, required XLS preamble fields, and template compliance expectations.
  • Summarize Amendment template section structure and placeholder rejection rules enforced by CI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


| Path | Purpose |
| ------------------------------------ | --------------------------------------------------------------------- |
| `XLS-NNNN-short-title/README.md` | One specification per standard (74 total) |
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

The project layout hard-codes “(74 total)”, but the repo already contains an additional nonconforming XLS directory (xls-0097-server-definitions-enhancements/) and the total will change over time. Suggest removing the exact count (or wording it as “one per directory under XLS-*/README.md”) to avoid the instructions becoming stale or misleading.

Suggested change
| `XLS-NNNN-short-title/README.md` | One specification per standard (74 total) |
| `XLS-NNNN-short-title/README.md` | One specification per standard (one per `XLS-*` directory) |

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +39
```
<pre>
xls: [number]
title: [max 44 chars, no "XLS" prefix or number]
description: [one full sentence]
author: [Name <email@example.com>, Name2 (@github-handle)]
category: [Amendment | System | Ecosystem | Meta]
status: [Draft | Final | Living | Deprecated | Stagnant | Withdrawn]
proposal-from: [URL to GitHub Discussions thread]
created: YYYY-MM-DD
updated: YYYY-MM-DD ← optional
implementation: [URL] ← optional; required for Final Amendment/System
requires: XLS-N, XLS-M ← optional
withdrawal-reason: [text] ← required if status is Withdrawn
</pre>
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

The preamble “copy/paste” example includes annotation text (e.g., “← optional/required”) inside the fenced block. If a contributor copies this verbatim, CI date validation will fail (e.g., updated: YYYY-MM-DD ← optional won’t match YYYY-MM-DD). Move the annotations outside the code block or convert them to prose so the snippet itself is valid when copied.

Copilot uses AI. Check for mistakes.
Comment on lines +67 to +69
For Amendment-category XLSes, `## 3. Specification` contains components as `### 3.N. ComponentType: \`Name\``subsections. Each component has numbered sub-subsections`#### 3.N.M.` and deeper.

### Ledger Entry (`### 3.N. Ledger Entry: \`Name\``)
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

Markdown formatting around the Amendment template heading examples is broken: the inline-code span uses \-escaped backticks (which render literally inside code spans) and there are missing spaces ("``subsections" / "sub-subsections`####"). Consider rewriting these examples using either double-backtick code spans (so literal backticks render correctly) or a fenced code block, and fix the missing spaces for readability.

Suggested change
For Amendment-category XLSes, `## 3. Specification` contains components as `### 3.N. ComponentType: \`Name\``subsections. Each component has numbered sub-subsections`#### 3.N.M.` and deeper.
### Ledger Entry (`### 3.N. Ledger Entry: \`Name\``)
For Amendment-category XLSes, `## 3. Specification` contains components as headings like ``### 3.N. ComponentType: `Name` `` subsections. Each component has numbered sub-subsections like ``#### 3.N.M.`` and deeper.
### Ledger Entry (example heading: ``### 3.N. Ledger Entry: `Name` ``)

Copilot uses AI. Check for mistakes.
Comment on lines +146 to +151
| Ledger Entry Fields | `Field Name \| Constant \| Required \| Internal Type \| Default Value \| Description` |
| Transaction Fields | `Field Name \| Required? \| JSON Type \| Internal Type \| Default Value \| Description` |
| Flags (both) | `Flag Name \| Flag Value \| Description` |
| RPC Request Fields | `Field Name \| Required? \| JSON Type \| Description` |
| RPC Response Fields | `Field Name \| Always Present? \| JSON Type \| Description` |
| Transaction Metadata | `Field Name \| Validated \| Always Present? \| Type \| Description` |
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

In the “Table Schemas” table, the column strings are wrapped in inline code and also escape pipes as \|. Inside an inline-code span the backslash is rendered literally, so this will display \| instead of |. To show the exact headings, prefer a fenced code block for each schema (or remove the outer code span and keep the \| escaping so it renders as literal | inside the table cell).

Suggested change
| Ledger Entry Fields | `Field Name \| Constant \| Required \| Internal Type \| Default Value \| Description` |
| Transaction Fields | `Field Name \| Required? \| JSON Type \| Internal Type \| Default Value \| Description` |
| Flags (both) | `Flag Name \| Flag Value \| Description` |
| RPC Request Fields | `Field Name \| Required? \| JSON Type \| Description` |
| RPC Response Fields | `Field Name \| Always Present? \| JSON Type \| Description` |
| Transaction Metadata | `Field Name \| Validated \| Always Present? \| Type \| Description` |
| Ledger Entry Fields | Field Name \| Constant \| Required \| Internal Type \| Default Value \| Description |
| Transaction Fields | Field Name \| Required? \| JSON Type \| Internal Type \| Default Value \| Description |
| Flags (both) | Flag Name \| Flag Value \| Description |
| RPC Request Fields | Field Name \| Required? \| JSON Type \| Description |
| RPC Response Fields | Field Name \| Always Present? \| JSON Type \| Description |
| Transaction Metadata | Field Name \| Validated \| Always Present? \| Type \| Description |

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants