From 32948cacc6fcf04e374880e4abb109c534b7a3b0 Mon Sep 17 00:00:00 2001 From: Chris Morgan Date: Thu, 13 Jan 2022 00:51:17 +1100 Subject: [PATCH] Add review for cfg-if v1.0.0 --- .../reviews/2022-01-package-jqCnIw.proof.crev | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/QE5OVlHZ4QyOcMqdjXhS1MgsoZHvUqxOHNZwyfpsDIU/reviews/2022-01-package-jqCnIw.proof.crev b/QE5OVlHZ4QyOcMqdjXhS1MgsoZHvUqxOHNZwyfpsDIU/reviews/2022-01-package-jqCnIw.proof.crev index e71be1a..84c715d 100644 --- a/QE5OVlHZ4QyOcMqdjXhS1MgsoZHvUqxOHNZwyfpsDIU/reviews/2022-01-package-jqCnIw.proof.crev +++ b/QE5OVlHZ4QyOcMqdjXhS1MgsoZHvUqxOHNZwyfpsDIU/reviews/2022-01-package-jqCnIw.proof.crev @@ -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. + confirms it’s unintended. + + Since the 1.0.0 release, there have been [a few commits + ]. 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 + + 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 ----- + -- 2.42.0