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

[David Blaikie via cfe-commits]

On Tue, Jan 17, 2017 at 12:57 AM Manuel Klimek <klimek at google.com> 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.
>
For approval it's important that that reaches the mailing list so it's
accounted for/visible. Otherwise it looks like code was sent for review and
then committed without review being performed which is not generally
accepted.

- Dave


>
> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170117/8e3bff90/attachment-0001.html>