This revision was automatically updated to reflect the committed changes.
Closed by commit rL292175: Update tools to use new getStyle API (authored by
amaiorano).
Changed prior to commit:
https://reviews.llvm.org/D28315?vs=84539&id=84611#toc
Repository:
rL LLVM
https://reviews.llvm.org/D283
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Lgtm. Thanks!
https://reviews.llvm.org/D28315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listi
amaiorano added a comment.
In https://reviews.llvm.org/D28315#647212, @ioeric wrote:
> In https://reviews.llvm.org/D28315#647154, @amaiorano wrote:
>
> > In https://reviews.llvm.org/D28315#647103, @ioeric wrote:
> >
> > > Let me know when broken tests are fixed and this patch (and the
> > > corr
ioeric added a comment.
In https://reviews.llvm.org/D28315#647154, @amaiorano wrote:
> In https://reviews.llvm.org/D28315#647103, @ioeric wrote:
>
> > Let me know when broken tests are fixed and this patch (and the
> > corresponding patch) is ready again for review. Also let me know if you
> >
amaiorano added a comment.
In https://reviews.llvm.org/D28315#647103, @ioeric wrote:
> Let me know when broken tests are fixed and this patch (and the corresponding
> patch) is ready again for review. Also let me know if you need any help.
I updated the two patches after rebasing on latest (no
amaiorano updated this revision to Diff 84539.
amaiorano added a comment.
Rebased changes on latest. No functional changes in this diff since last.
All tests pass (on Windows).
https://reviews.llvm.org/D28315
Files:
change-namespace/ChangeNamespace.cpp
clang-apply-replacements/tool/ClangAp
ioeric added a comment.
Let me know when broken tests are fixed and this patch (and the corresponding
patch) is ready again for review. Also let me know if you need any help.
Comment at: change-namespace/ChangeNamespace.cpp:892
+ llvm::errs() << llvm::toString(Style.takeE
amaiorano added inline comments.
Comment at: change-namespace/ChangeNamespace.cpp:892
+ llvm::errs() << llvm::toString(Style.takeError()) << "\n";
+ continue;
+}
amaiorano wrote:
> amaiorano wrote:
> > ioeric wrote:
> > > I'd still like to apply rep
amaiorano added inline comments.
Comment at: change-namespace/ChangeNamespace.cpp:892
+ llvm::errs() << llvm::toString(Style.takeError()) << "\n";
+ continue;
+}
amaiorano wrote:
> ioeric wrote:
> > I'd still like to apply replacements that are not
amaiorano updated this revision to Diff 83952.
amaiorano added a comment.
Fixed code in ClangApplyReplacementsMain.cpp so that we only handle the
Expected
amaiorano added a comment.
In https://reviews.llvm.org/D28315#641032, @amaiorano wrote:
> In https://reviews.llvm.org/D28315#639559, @ioeric wrote:
>
> > I ran `ninja check-all` with https://reviews.llvm.org/D28081 and
> > https://reviews.llvm.org/D28315 (in Linux), and some lit tests failed,
>
amaiorano added a comment.
In https://reviews.llvm.org/D28315#639559, @ioeric wrote:
> I ran `ninja check-all` with https://reviews.llvm.org/D28081 and
> https://reviews.llvm.org/D28315 (in Linux), and some lit tests failed, namely:
>
> Clang :: Format/style-on-command-line.cpp
This was the
amaiorano added a comment.
In https://reviews.llvm.org/D28315#639559, @ioeric wrote:
> I ran `ninja check-all` with https://reviews.llvm.org/D28081 and
> https://reviews.llvm.org/D28315 (in Linux), and some lit tests failed, namely:
>
> Clang :: Format/style-on-command-line.cpp
> Clang Tools
ioeric added a comment.
I ran `ninja check-all` with https://reviews.llvm.org/D28081 and
https://reviews.llvm.org/D28315 (in Linux), and some lit tests failed, namely:
Clang :: Format/style-on-command-line.cpp
Clang Tools :: clang-apply-replacements/basic.cpp
Clang Tools :: clang-apply-rep
amaiorano added a comment.
@ioeric Okay, check-clang-tools passes with my changes.
FYI, tests were originally failing for me (without my changes) because I use
Git, and it's configured to convert line endings to Windows (crlf) on checkout,
but some of the tools tests specify extract offsets int
amaiorano added a comment.
In https://reviews.llvm.org/D28315#636888, @hokein wrote:
> In https://reviews.llvm.org/D28315#636821, @amaiorano wrote:
>
> > Is there a way to run tests without ninja? I'm on Windows. If not, I'll use
> > my Linux VM.
>
>
> It is not related to the build system. Ther
hokein added a comment.
In https://reviews.llvm.org/D28315#636821, @amaiorano wrote:
> Is there a way to run tests without ninja? I'm on Windows. If not, I'll use
> my Linux VM.
It is not related to the build system. There should be a `check-clang-tools`
project, you can run tests by building
amaiorano added a comment.
In https://reviews.llvm.org/D28315#636670, @hokein wrote:
> In https://reviews.llvm.org/D28315#635865, @amaiorano wrote:
>
> > I made these changes, and they build, but I didn't really test them. Are
> > there unit tests for these tools that I can run?
>
>
> If you use
hokein added a comment.
In https://reviews.llvm.org/D28315#635865, @amaiorano wrote:
> I made these changes, and they build, but I didn't really test them. Are
> there unit tests for these tools that I can run?
If you use ninja build, you can run `ninja check-clang-tools` to run all tests.
h
malcolm.parsons added subscribers: cfe-commits, malcolm.parsons.
malcolm.parsons added a comment.
A mailing list cannot review.
https://reviews.llvm.org/D28315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mai
amaiorano added a comment.
I made these changes, and they build, but I didn't really test them. Are there
unit tests for these tools that I can run?
https://reviews.llvm.org/D28315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://list
amaiorano created this revision.
amaiorano added reviewers: ioeric, cfe-commits, klimek, djasper.
See https://reviews.llvm.org/D28081 for the changes to getStyle
This change will be committed right after https://reviews.llvm.org/D28081.
https://reviews.llvm.org/D28315
Files:
change-namespace
22 matches
Mail list logo
