[libcxx-commits] [PATCH] D118800: [libc++] Normalize all our '#pragma GCC system_header', and regression-test.

[Joe Loser via Phabricator via libcxx-commits]

jloser added inline comments.


================
Comment at: libcxx/test/libcxx/lint/lint_headers.sh.py:46
+                okay = False
+                print('FAILED TO FIND #pragma GCC system_header in libcxx/include/%s!' % pretty(fname))
+
----------------
Quuxplusone wrote:
> jloser wrote:
> > Nit: I think we rely on `python3` exclusively now, so we could make this a Python3 f-string which I think is nicer. Thoughts? I hear the argument that this is consistent with the rest of the file (because it was mostly written when we only supported Python2).
> > I hear the argument that this is consistent with the rest of the file (because it was mostly written when we only supported Python2).
> 
> Actually it was written very recently, but by an old curmudgeon who likes `printf`. Between these options
> * `print('foo/%s!' % pretty(x))`
> * `print('foo/{}!'.format(pretty(x)))`
> * `print(f'foo/{pretty(x)}!')`
> I'm personally inclined to favor #1, just for readability's sake... although I suspect I'm on the losing side of history on this one. ;)
Ah yes, how could I forget it was written by you last month? :)

I prefer option 3 with the f-string, but maybe I'm just a new-school Python guy. If we care to update these kinds of things, we can use some tool (maybe Black or Ale or something) to fix (and maybe even enforce if we care) "modern" use, so don't feel obligated to do anything in this PR. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118800