[PATCH] D72658: [llvm-nm] Don't report "no symbols" error for files that contain symbols

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-nm/X86/nm-no-symbols-local-only.yaml:13
+    Type:            STT_FILE
+    Index:           SHN_ABS
+
----------------
sbc100 wrote:
> grimar wrote:
> > nit: we often remove the excessive identation. It
> > makes the output to be a bit more readable.
> The prevailing style in this directory seems to be to preserve the yaml with indentation and alignment.  I'm not sure I agree that its more readable either.    So if its OK with you I'll leave this as is for this change and you and propose an widespread change if you feel strongly.
I'm also of the opinion that new tests are better without the extra whitespace for readability. However, I don't think it's worth the noise to fix up the existing tests - I prefer a more organic migration from one style to the other, i.e. as you change the YAML in an area, update it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72658