Ericson2314 marked an inline comment as done.
Ericson2314 added inline comments.
================
Comment at: clang/CMakeLists.txt:26
"--src-root"
- "--cmakedir")
+ "--cmakedir"
+ "--bindir"
----------------
Ericson2314 wrote:
> beanz wrote:
> > Ericson2314 wrote:
> > > beanz wrote:
> > > > I assume these are re-arranged to match what you're doing in lld. Is
> > > > that correct?
> > > >
> > > > Generally it is preferred to do this kind of non-functional
> > > > restructuring in a separate commit from the commit with a functional
> > > > change to make it easier to review the functional changes.
> > > >
> > > > This patch as-is seems to have cleanup to clang, and a functional
> > > > change for lld. Those should likely be two separate commits.
> > > Happy to split up.
> > >
> > > The clang changes I wouldn't even say make it "better" per say, I am just
> > > reording things to put the stuff LLD also goes first for sake diffing the
> > > two. I thought that might look lke a noisy and pointless patch on its
> > > own, but I am happy to split it up if you still think it's a good idea.
> > More smaller patches is generally the LLVM philosophy, so I do think it
> > makes sense to split it up into an NFC patch for clang, and a separate
> > change for lld.
> Sure. Done in D116492.
Oops, that's this one! I meant D116548.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116492/new/
https://reviews.llvm.org/D116492
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
