Conversation
|
Would be nice if we could avoid adding |
| - [ ] Use `VoteBehavior::DefaultNo` (validators must explicitly vote for it) | ||
|
|
||
| #### 3. Code Implementation | ||
| - [ ] Implement new functionality (transaction type, ledger entry, etc.) |
There was a problem hiding this comment.
At the moment AI agents are not amazing at performing long implementation jobs without step-by-step plans. So Implement new functionality is a bit vague for the AI Agent's checklist. Maybe we should add more fine grained list. Example:
- Breakdown the changes to be made into tasks and sub-tasks.
- Create a task list of changes to be made for the feature/fix in
featureNameTasks.jsonfile. IncludeTitle,Description,Files to touchandDependencies on other tasks, for each task and sub-task. - Add a field called 'Status' to each task and sub-task. This field will be set to
Incompleteat the beginning. - Update the field with
In progress, when working on the item and then toCompletewhen finished. - Go through the task list, one item at a time, and in the order of dependencies (least to most), and implement the task. Update status.
- Add comments for each change
- Add proper error handling.
- Once the task-list is complete:
- Make sure the code builds successfully.
- Reset the status of all the tasks and subtasks (to
Incomplete) and make another pass at the task list. Confirming if the changes have been implemented correctly and completely.
There was a problem hiding this comment.
A Build skill would be very useful addition.
There was a problem hiding this comment.
I noticed that the # Task Management principal covers this to some extent.
| } else { | ||
| // Legacy behavior | ||
| } | ||
| break; |
There was a problem hiding this comment.
A default case is expected by the compiler. Skipping it here might force AI agent to skip it as well.
| - Never mark a task complete without proving it works | ||
| - Diff behavior between main and your changes when relevant | ||
| - Ask yourself: "Would a staff engineer approve this?" | ||
| - Run tests, check logs, demonstrate correctness |
There was a problem hiding this comment.
Might be a good addition.
- Confirm before adding a new external dependency.
- Check if an exiting module can be reused.
- Make sure the new code is modular, extensible and testable.
| - [ ] Add feature gate check in preflight: | ||
| ```cpp | ||
| if (!env.current()->rules().enabled(feature{Name})) | ||
| { | ||
| return temDISABLED; | ||
| } | ||
| ``` |
There was a problem hiding this comment.
If you're adding a new transaction gated on an amendment, you only need to define specify the amendment in transactions.macro. If you're adding a new feature to an existing transaction, use checkExtraFeatures if possible.
| - The `develop` branch has removed copyright/license comment blocks from source files. If a conflict is just about the copyright header, take `develop`'s version (no header). | ||
|
|
||
| ### File Moves | ||
| - Many files have been moved from `src/xrpld/` to `src/libxrpl/` or from `src/xrpld/` headers to `include/xrpl/`. If git shows a file as "deleted in develop" but it exists at a new path, delete the old-path file (`git rm`) and keep the new-path version. |
There was a problem hiding this comment.
Except that if that causes a conflict, it's possible that something changed in the develop version that needs to be replicated in the feature branch.
|
|
||
| **Resolution strategy:** | ||
| 1. Keep all of `develop`'s transactions. | ||
| 2. Add your feature's transactions at the bottom with the next available number. |
There was a problem hiding this comment.
Instead of the next available, leave a 2-5 item gap, ensuring there are no collisions. This allows transaction "groups" to be expanded, but stay together.
4 participants
High Level Overview of Change
Context of Change
Type of Change
.gitignore, formatting, dropping support for older tooling)API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)