User Details
- User Since
- Jan 31 2016, 7:15 AM (260 w, 1 d)
Fri, Jan 22
LGTM, thanks for cleaning this up!
Superseded by D95188
Superseded by D95188
LGTM
Overall this looks good. I wonder if abstracting the ExternalRedirect as a small wrapper class around a SmallString would help. There's a few operations that are repeated, like the example below, and it could also house the logic that's currently in the lambda.
Thu, Jan 21
- Implement makeCanonical using canonicalize
- Add missing call to makeCanonical is isLocal
LGTM but I'd like @teemperor to have a look too as he authored the original change.
Wed, Jan 20
LGTM!
I replied before I actually tried to understand what your'e trying to achieve, and it's still not entirely clear to me. I think what you need is something similar like python-typemaps.swig which helps swig understand char** types and such.
@jingham should weigh in here, but I think we kind of guarantee (at least informally) that the command options to be stable. Is there an alternative way to fix this without breaking the existing option?
Thanks!
LGTM with the inline comment addressed.
Tue, Jan 19
Ship it
LGTM
Use SmartMutex and SmartScopedLock.
Sounds like something to tablegen ;-) Joking aside, this seems easy enough to auto-generate with clang tooling, so LGTM as a temporary measure.
Mon, Jan 18
We guarantee that the SB API is ABI stable so you cannot change the signature of existing functions. Would an overload do the trick?
Cool. That was one of the reasons I suggested having the SB API documentation integrated with the rest of the docs.
LGTM
Fri, Jan 15
Personally I'd just like to get rid of the shouldUseExternalFS and let the underlying FS take care of it, which I believe would better fit the abstraction but would be a change in behavior (as pointed out in https://reviews.llvm.org/D65677#inline-604694).
Thu, Jan 14
LGTM. Thanks for driving this!
ping
Wed, Jan 13
Tue, Jan 12
Address @avl's review comments.
Mon, Jan 11
Address code review feedback
Sun, Jan 10
LGTM
LGTM for LLDB