Skip to content

fix: Fix a rounding error at the Number::maxRep cusp#7051

Draft
ximinez wants to merge 5 commits intodevelopfrom
ximinez/number-fix-maxrepcusp
Draft

fix: Fix a rounding error at the Number::maxRep cusp#7051
ximinez wants to merge 5 commits intodevelopfrom
ximinez/number-fix-maxrepcusp

Conversation

@ximinez
Copy link
Copy Markdown
Collaborator

@ximinez ximinez commented Apr 29, 2026

High Level Overview of Change

If the mantissa result of an operation is maxRep, and should round up, it rounds down instead.

Context of Change

API Impact

@ximinez ximinez changed the title Fix a rounding error at the Number::maxRep cusp fix: Fix a rounding error at the Number::maxRep cusp Apr 30, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 3, 2026

This PR has conflicts, please resolve them in order for the PR to be reviewed.

@godexsoft
Copy link
Copy Markdown
Contributor

We recently merged a refactor to develop that enables clang-tidy's readability-identifier-naming. Your branch now has heavy conflicts that are largely mechanical. Below is a workflow that aligns your branch's naming with develop before merging, which should minimize the merge conflicts.

One-time setup

If you don't already have clang-tidy working in your env, on macOS:

brew install llvm@21
# Follow brew's hint to put $(brew --prefix llvm@21)/bin on PATH so run-clang-tidy is found.

Workflow on your branch (before merging develop)

1. Grab the new .clang-tidy from develop without pulling anything else. Sync your fork on GitHub first, then:

git remote -v   # should show 'upstream' among others; if not:
# git remote set-url upstream git@github.com:XRPLF/rippled.git
git fetch upstream
git checkout upstream/develop -- .clang-tidy

2. Reconfigure conan/cmake so compile_commands.json is fresh.

3. Apply renames for the files modified in your PR:

git diff --name-only $(git merge-base HEAD upstream/develop) HEAD \
  | grep -E '\.(cpp|h|hpp|ipp)$' \
  | xargs run-clang-tidy -p build -fix -allow-no-checks
# or -p .build, or whatever your build dir is called

4. Build + test, then commit as a single dedicated commit:

cmake --build build -j8
git commit -am "refactor: Align identifier naming with develop"

5. Now merge develop:

git merge upstream/develop

Extra

Run clang-tidy once more after the merge to catch any stragglers introduced from develop's side:

run-clang-tidy -p build -fix -allow-no-checks src tests
# or -p .build, or whatever your build dir is called

- Fix is only enabled if SAV or LP is also enabled
- Basically a lazy way to avoid breaking AMM unit tests
@ximinez ximinez force-pushed the ximinez/number-fix-maxrepcusp branch from fe3ab2f to 9801e7c Compare May 5, 2026 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants