<div dir="ltr">Please upload your latest revision to Phabricator so that I can submit on behalf of you.</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 15, 2015 at 10:10 PM, Denis Protivensky <span dir="ltr"><<a href="mailto:dprotivensky@accesssoftek.com" target="_blank">dprotivensky@accesssoftek.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I see everyone has accepted the revision.<br>
How should I merge it as I don't have write permissions in the repo? Can someone help me with this please?<br>
<span class=""><br>
<br>
================<br>
Comment at: lib/ReaderWriter/ELF/ARM/ARMExecutableWriter.h:9<br>
@@ +8,3 @@<br>
+//===----------------------------------------------------------------------===//<br>
+#ifndef ARM_EXECUTABLE_WRITER_H<br>
+#define ARM_EXECUTABLE_WRITER_H<br>
----------------<br>
</span><span class="">ruiu wrote:<br>
> ifdef guard should include the path name.<br>
><br>
>   #ifndef LLD_READER_WRITER_ELF_ARM_ARM_EXECUTABLE_WRITER_H<br>
><br>
</span>Good.<br>
<span class=""><br>
================<br>
Comment at: lib/ReaderWriter/ELF/ARM/ARMExecutableWriter.h:39<br>
@@ +38,3 @@<br>
+  ARMLinkingContext &_context;<br>
+  ARMTargetLayout<ELFT> &_ARMLayout;<br>
+};<br>
----------------<br>
</span>ruiu wrote:<br>
> _ARM -> _arm<br>
Fixed.<br>
<span class=""><br>
================<br>
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.h:34<br>
@@ +33,3 @@<br>
+private:<br>
+  ARMTargetLayout<ARMELFType> &_ARMTargetLayout;<br>
+};<br>
----------------<br>
</span><span class="">ruiu wrote:<br>
> A name beginning with '_' followed by an uppercase character is reserved to the implementation (just like __), so please don't use such name. s/_ARMTargetLayout/_armTargetLayout/.<br>
</span>Good, didn't know about that.<br>
<span class=""><br>
================<br>
Comment at: lib/ReaderWriter/ELF/ARM/ARMTarget.h:1<br>
@@ +1,2 @@<br>
+//===--------- lib/ReaderWriter/ELF/ARM/ARMTarget.h -----------------------===//<br>
+//<br>
----------------<br>
</span><span class="">ruiu wrote:<br>
> Why do we need this almost-empty header? Can't we directly include ARMLinkingContext.h?<br>
</span>That's how the structure of files is organized.<br>
Targets.h includes platform-specific *Target.h file, and it in turn includes its linking context.<br>
<div class="HOEnZb"><div class="h5"><br>
<a href="http://reviews.llvm.org/D6716" target="_blank">http://reviews.llvm.org/D6716</a><br>
<br>
EMAIL PREFERENCES<br>
  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br>
</div></div></blockquote></div><br></div>