[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

[Manikishan Ghantasala via Phabricator via cfe-commits]

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