[PATCH] D75237: [APFloat] Overload comparison operators
Ehud Katz via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 6 03:51:12 PST 2020
ekatz added a comment.
Generally it LGTM.
But this change may be split into 2 as well (like the `neg` change).
I know that some times you see patches that change and add a lot of code (in the same patch), while in other cases they split into smaller ones. Both approaches are valid, but I think the better one is to split (if possible); especially in cases (like this) that change apply to files, from separate projects/modules, outside the one in which the new API is added.
So, I suggest split it into 2, where one adds the new operators and tests; and the other changes the //compares// in the different locations.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75237/new/
https://reviews.llvm.org/D75237
More information about the llvm-commits
mailing list