[PATCH] D95017: [clang-format] add case aware include sorting
MyDeveloperDay via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 25 14:18:19 PST 2021
MyDeveloperDay added inline comments.
================
Comment at: clang/docs/ClangFormatStyleOptions.rst:2286
+**IncludeSortAlphabetically** (``bool``)
+ Specify if sorting should be done in an alphabetical and
+ case sensitive fashion.
----------------
Are you sure `IncludeSortAlphabetically` expresses what you mean? Once these things get released they cannot be changed easily.
If you were not sure of a new option, in my view we should slow down and make sure we have the correct design, for example you could have used a enum and it might have given you more possibility for greater flexibility
```
enum IncludeSort
{
CaseInsensitive
CaseSensitive
}
```
Please give people time to re-review your changes before we commit, especially if they've taken the time to look at your review in the first place. Just saying.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95017/new/
https://reviews.llvm.org/D95017
More information about the cfe-commits
mailing list