[PATCH] D88827: [llvm-objcopy][NFC] Move core implementation of llvm-objcopy into separate library.

Alexey Lapshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 8 05:32:34 PDT 2020


avl added a comment.

> 1, 2 - perhaps, this can be refactored / reorganized / better documented, but it would be great to have it done before the introduction of the library.

if possible - I propose to do these points as next patches after this(to make some progress on it). If not - I would work on preliminary patches.



================
Comment at: llvm/lib/ObjCopy/COFF/Object.cpp:1
 //===- Object.cpp ---------------------------------------------------------===//
 //
----------------
alexshap wrote:
> avl wrote:
> > jhenderson wrote:
> > > Whilst you're moving this and the equivalent files for other formats around, could you please rename them to be obvious from the filename which format they are for (same goes for their headers), please? For example, COFF/Object.cpp -> COFF/COFFObject.cpp. The reason for this is that when using the Visual Studio IDE, all the "Object.cpp" files end up listed next to each other in the file browser, and the only way of figuring out which is which is by opening them and seeing.
> > ok.
> @jhenderson, I'm sorry to disagree, but renaming files this way doesn't seem to be a good idea and the provided justification doesn't appear to be sufficient.
> Since this file contains the implementation of what's declared in Object.h I would strongly prefer to have it named Object.cpp given it is already located in the corresponding folder. Visual Studio IDE might have some peculiarities but having consistent naming is important, adding such prefixes doesn't seem to be a good approach. 
Would it be OK, If both of the files would be renamed Object.h -> COFFObject.h and Object.cpp->COFFObject.cpp ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88827/new/

https://reviews.llvm.org/D88827



More information about the llvm-commits mailing list