[PATCH] D28548: Improve include fixer's ranking by taking the paths into account.

Benjamin Kramer via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 17 02:43:32 PST 2017


If we add all state transitions it will only create noise. I will
teach myself to always write "lg" into the text field when approving a
change instead.

On Tue, Jan 17, 2017 at 9:57 AM, Manuel Klimek via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> It's by design. Do we want to change this? I always had the impression
> turning on the state changes for the list is a bit noisy, but I'm happy if
> that's not the general sentiment.
>
>
> On Mon, Jan 16, 2017, 11:07 PM Benjamin Kramer <benny.kra at gmail.com> wrote:
>>
>> I got an email where it says that I accepted the revision. Looks like
>> phab didn't add cfe-commits to the list of recipients though :(
>>
>> On Mon, Jan 16, 2017 at 6:43 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> > Looks like Ben signed off on this on Phab - but the email didn't go to
>> > the
>> > list (making this look like code was sent for review, then committed,
>> > without review/approval happening)
>> >
>> > Ben: I think Phab doesn't send mail for an approval with no text, so at
>> > least as a workaround you can write something in the comments section
>> > when
>> > approving (people often write "LGTM" or similar) to ensure the approval
>> > is
>> > reflected on the mailing list.
>> >
>> > On Wed, Jan 11, 2017 at 1:59 AM Manuel Klimek via Phabricator via
>> > cfe-commits <cfe-commits at lists.llvm.org> wrote:
>> >>
>> >> klimek created this revision.
>> >> klimek added a reviewer: bkramer.
>> >> klimek added a subscriber: cfe-commits.
>> >>
>> >> Instead of just using popularity, we also take into account how similar
>> >> the
>> >> path of the current file is to the path of the header.
>> >> Our first approach is to get popularity into a reasonably small scale
>> >> by
>> >> taking
>> >> log2 (which is roughly intuitive to how humans would bucket
>> >> popularity),
>> >> and
>> >> multiply that with the number of matching prefix path fragments of the
>> >> included
>> >> header with the current file.
>> >> Note that currently we do not take special care for unclean paths
>> >> containing
>> >> "../" or "./".
>> >>
>> >>
>> >> https://reviews.llvm.org/D28548
>> >>
>> >> Files:
>> >>   include-fixer/IncludeFixer.cpp
>> >>   include-fixer/SymbolIndexManager.cpp
>> >>   include-fixer/SymbolIndexManager.h
>> >>   include-fixer/tool/ClangIncludeFixer.cpp
>> >>   test/include-fixer/Inputs/fake_yaml_db.yaml
>> >>   test/include-fixer/ranking.cpp
>> >>
>> >> _______________________________________________
>> >> cfe-commits mailing list
>> >> cfe-commits at lists.llvm.org
>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>


More information about the cfe-commits mailing list