<div dir="ltr"><br><div>You're okay to submit.  I will probably move the declarations around slightly in a separate patch.  :-)</div><div><br></div><div>  -DeLesley</div><div><br></div></div><div class="gmail_extra"><br>
<br><div class="gmail_quote">On Tue, Sep 2, 2014 at 2:19 PM, Ed Schouten <span dir="ltr"><<a href="mailto:ed@80386.nl" target="_blank">ed@80386.nl</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Delesley, Aaron,<br>
<div class=""><br>
On 2 September 2014 22:44, Delesley Hutchins <<a href="mailto:delesley@google.com">delesley@google.com</a>> wrote:<br>
> (1) FactEntry needs a virtual destructor to avoid memory leaks.<br>
<br>
</div>Done!<br>
<div class=""><br>
> (2) I would like the different FactEntry classes to be declared together (at<br>
> the top of the file), rather than split as they are currently.<br>
<br>
</div>The problem is that the *LockableFactEntry classes depend on FactSet<br>
and FactManager, whereas the FactSet and FactManager depend on the<br>
FactEntry.<br>
<br>
I could possibly declare them together, but in that case, the<br>
implementations of handle*() would need to go elsewhere. I'm usually<br>
not a big fan of that approach. Be sure to let me know if you'd prefer<br>
that anyway.<br>
<div class=""><br>
On 2 September 2014 22:49, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
> I would not reformat the entire file at this point (that tends to be<br>
> pretty noisy for little gain). Instead, you can run clang-format over<br>
> the patch file itself. There's a pretty simple script for doing this<br>
> described in the documentation:<br>
> <a href="http://clang.llvm.org/docs/ClangFormat.html" target="_blank">http://clang.llvm.org/docs/ClangFormat.html</a><br>
<br>
</div>That seems to work perfectly. I've just reformatted the code. Updated patch:<br>
<br>
<a href="http://80386.nl/pub/20140902-clang-scoped_lockable-2.txt" target="_blank">http://80386.nl/pub/20140902-clang-scoped_lockable-2.txt</a><br>
<br>
Be sure to let me know if there's anything else that you'd like to see<br>
differently. I'll probably push in the change some time tomorrow<br>
morning/afternoon.<br>
<div class="HOEnZb"><div class="h5"><br>
Thanks,<br>
--<br>
Ed Schouten <<a href="mailto:ed@80386.nl">ed@80386.nl</a>><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br>DeLesley Hutchins | Software Engineer | <a href="mailto:delesley@google.com" target="_blank">delesley@google.com</a> | 505-206-0315<br>
</div>