<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Aug 20, 2013, at 11:24 PM, Michael Gottesman <<a href="mailto:mgottesman@apple.com">mgottesman@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=us-ascii"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Looks much better. One last nit then LGTM. I understand why you would make {add,ignore}_llvm_tool_subdirectory macros since they are so small,</div></blockquote><div><br></div><div>Actually these need to be macros so they can introduce the LLVM_IMPLICIT_PROJECT_IGNORE variable.</div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"> but do you have any reason why you made add_llvm_implicit_external_projects a macro. It seems to me to be more of a function since it:<div><br></div><div>1. Takes no arguments.</div><div>2. Sets variables which are used only locally suggesting that you want to have the local scope of a function instead of the global scope of a macro.</div></div></blockquote><div><br></div><div>You're right; changed add_llvm_implicit_external_projects to a function and attached new patch.</div><div><br></div><div></div></div></body></html>