Manikishan added a comment. In D64695#1606256 <https://reviews.llvm.org/D64695#1606256>, @rdwampler wrote:
> In D64695#1605676 <https://reviews.llvm.org/D64695#1605676>, @Manikishan > wrote: > > > In D64695#1590948 <https://reviews.llvm.org/D64695#1590948>, @Manikishan > > wrote: > > > > > In D64695#1589818 <https://reviews.llvm.org/D64695#1589818>, @lebedev.ri > > > wrote: > > > > > > > In D64695#1589740 <https://reviews.llvm.org/D64695#1589740>, > > > > @Manikishan wrote: > > > > > > > > > In D64695#1589508 <https://reviews.llvm.org/D64695#1589508>, > > > > > @lebedev.ri wrote: > > > > > > > > > > > Is there sufficient test coverage as to what happens if > > > > > > `SortPriority` is not set? > > > > > > > > > > > > > > > If SortPriority is not set, the Includes will be grouped without > > > > > sorting, > > > > > > > > > > > > Let me rephrase - for the exiting `.clang-format`s, that don't > > > > currently specify `SortPriority`, > > > > this introduction of `SortPriority` should not change the header > > > > handling. > > > > Is that the case, and if so is there sufficient test coverage for that? > > > > > > > > > I got your idea now. > > > No, there is no test coverage for that case, and with the current patch > > > they have to add SortPriority. > > > To avoid this shall I set SortPriority as Priority as default if it is > > > not defined? I think that will fix the issue. > > > > > > any reviews on it ? > > > That's sounds like it will work. Can you add some additional test cases > around this in `SortIncludesTest.cpp`. Also, adding a test case specifically > for sorting the NetBSD headers would be good. Sorry for the delay, I am facing issues with "NoCrash_Bug34236" will update the patch once I am able to fix it. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64695/new/ https://reviews.llvm.org/D64695 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
