Add review for cfg-if v1.0.0
authorChris Morgan <me@chrismorgan.info>
committerChris Morgan <me@chrismorgan.info>
QE5OVlHZ4QyOcMqdjXhS1MgsoZHvUqxOHNZwyfpsDIU/reviews/2022-01-package-jqCnIw.proof.crev

index e71be1a15c007e23b1c59f1f446e4105afe03a88..84c715de6aa291df8655b8554b5d5d8e72df4e75 100644 (file)
@@ -391,3 +391,59 @@ comment: |-
 C30SLGF6ibx4QaLXo6uazHuIio1iP_rSmab7Vo6nLEeFZfuoaFTqiaMhPYflhvcGHEu6jImRBBIvmworq13bBg
 ----- END CREV PROOF -----
 
+----- BEGIN CREV PROOF -----
+kind: package review
+version: -1
+date: "2022-01-13T00:42:12.174512813+11:00"
+from:
+  id-type: crev
+  id: QE5OVlHZ4QyOcMqdjXhS1MgsoZHvUqxOHNZwyfpsDIU
+  url: "https://git.chrismorgan.info/crev-proofs"
+package:
+  source: "https://crates.io"
+  name: cfg-if
+  version: 1.0.0
+  revision: e60fa1efeab0ec6e90c50d93ec526e1410459c23
+  digest: YmRVto2mw7U7GJhrOe13k_5Sgg9TcQWK1NuLN2Qiz5E
+review:
+  thoroughness: high
+  understanding: high
+  rating: positive
+comment: |-
+  This is a robust crate, clear, simple and well-documented within and without,
+  powered by a well-designed tt gobbler, and written by Alex Crichton of
+  exemplar reputation.
+  
+  I’m quite fond of the ``$(if … { … })else* else { … }`` trick, using the
+  keyword ``else`` as the repetition delimiter. I knew you could put a variety
+  of things there, not just characters like comma or semicolon, but I hadn’t
+  connected the dots that that would extended to keywords. Hey, you can even
+  use *string literals*! Crazy stuff. Anyway, that’s a neat way of keeping the
+  public API to two rules rather than three as I would probably have done.
+  
+  I found one surprise in this: ``cfg_if! { else { true } }`` works, yielding
+  true, because that repetition I just showed used ``*`` instead of ``+``.
+  It’s not covered by the test suite, but it’s harmless.
+  <https://github.com/alexcrichton/cfg-if/issues/23> confirms it’s unintended.
+  
+  Since the 1.0.0 release, there have been [a few commits
+  <https://github.com/alexcrichton/cfg-if/commits/main>]. Most are minor
+  trivialities (yeah, I’ll even call Dependabot that, though I hate it and it’s
+  not even useful here because this crate has no dependencies), but
+  <https://github.com/alexcrichton/cfg-if/commit/349913689695130c2c63d1075e57b0917c98ee6e>
+  is significant and very mildly concerning: as well as twiddling some
+  formatting (for the good), it changes that ``*`` to ``+`` to deny bare else,
+  which is a breaking change, for unlikely but reasonable and plausible code.
+  I’ve commented on #23 and #39 to this effect; we’ll see what comes of it.
+  (It could be reverted, it could be ignored, it could be released as 2.0.0.)
+  
+  There are a couple of extraneous files in the package (.gitignore,
+  .github/workflows/main.yml), but they’re small enough that I won’t complain.
+  
+  I think this crate is overused and most places it’s used would be better
+  served by writing the negations themselves, which are often no longer and
+  often clearer.
+----- SIGN CREV PROOF -----
+ex_TABmVr_CTEY6B5C-NDKk7ciKdCK1bLf6ltjpXyoVknvyAC2AnRjA5rlPX_5ORQMeHmzJ2UxWUR0RdgBjYAQ
+----- END CREV PROOF -----
+