[PATCH] D99055: [llvm-objcopy] Refactor CopyConfig structure.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 29 01:03:58 PDT 2021


jhenderson resigned from this revision.
jhenderson added a comment.

Thanks for the update. I do have some concerns about the multiple inheritance stuff going on, if I'm honest, as usually this is a design smell, but I haven't got the time right now to think about it. Unfortunately, I'm not likely to get more of a chance to later this week either, as from Thursday I'm off for just over two weeks.

I do still think you should normalise the error messages in a separate prerequisite change. Small changes are good, especially if it helps make something else a pure refactor.

If I don't come back around to this before I go, please feel free to go with the approval of others. I'm resigning from reviewing to remove my "request changes" flag, but I may come back to it later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99055



More information about the llvm-commits mailing list