[PATCH] D130458: [llvm-objcopy] Support --{,de}compress-debug-sections for zstd

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 27 13:27:37 PDT 2022


MaskRay added a comment.

Yes!

  compression::Param P;
  P.Format = CompressionType;
  compression::compress(P, OriginalData, CompressedData);

is a lightweight use of the compression interface. I could move back the previous version which uses the slightly verbose `switch`, then this patch will have no dependency on D130506 <https://reviews.llvm.org/D130506>.

@ckissane What I want to demonstrate in this patch is that we should use multiple patches adding zstd support to different components, instead of having a monolithic D130516 <https://reviews.llvm.org/D130516>.

- https://gregoryszorc.com/blog/2020/01/07/problems-with-pull-requests-and-how-to-fix-them/ "Previous studies have found that the number of useful comments decreases and the review latency increases as the size of the change increases."
- When we add zstd support to various components, they should have dedicated tests.
- Each component is cared by its maintainers/regular reviewers/users. Having a refactoring is fine, but bundling zstd features in the huge patch may surprise them. There are many factors to consider, including what to do when zstd support is unavailable.
- Many components don't necessarily migrate to zstd.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130458



More information about the llvm-commits mailing list