From ff2cb2fd4ed1c80dd14304367dafaf1287f3eead Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Christian=20Gr=C3=BCnhage?= Date: Mon, 27 Jun 2022 22:44:05 +0200 Subject: [PATCH] Add review for yap v0.7.2 --- .../reviews/2022-06-package-0Oh57g.proof.crev | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/YdnEoYtqvbBGv0hhENLDUYmc3tNfm5V5NIG5hCovHyM/reviews/2022-06-package-0Oh57g.proof.crev b/YdnEoYtqvbBGv0hhENLDUYmc3tNfm5V5NIG5hCovHyM/reviews/2022-06-package-0Oh57g.proof.crev index f0d1711..1f0c6a4 100644 --- a/YdnEoYtqvbBGv0hhENLDUYmc3tNfm5V5NIG5hCovHyM/reviews/2022-06-package-0Oh57g.proof.crev +++ b/YdnEoYtqvbBGv0hhENLDUYmc3tNfm5V5NIG5hCovHyM/reviews/2022-06-package-0Oh57g.proof.crev @@ -23,3 +23,69 @@ comment: |- mJagAUfgDd8cusPe6ha009XxnQ2fCLzHl17T3gLpebo1RgIWmW6b41oezwkxbQeRh2i4Py7m1olDvu5fUSPsBg ----- END CREV PROOF ----- +----- BEGIN CREV PROOF ----- +kind: package review +version: -1 +date: "2022-06-27T22:44:05.352091075+02:00" +from: + id-type: crev + id: YdnEoYtqvbBGv0hhENLDUYmc3tNfm5V5NIG5hCovHyM + url: "https://git.jcg.re/jcgruenhage/crev-proofs.git" +package: + source: "https://crates.io" + name: yap + version: 0.7.2 + revision: 6ee4dfc176a5690063e391603b2b6ad2bd5b2514 + digest: K5Fu1zb_agTCEM2nYx2SyUDzBPRJ4EAFi8aK1eubGMM +review: + thoroughness: high + understanding: high + rating: positive +alternatives: + - source: "https://crates.io" + name: nom +comment: |- + Simple enough parser combinator library that comes with most of the bells and + whistles I was looking for. I've noticed a few minor things that could use + improving, but nothing critical. + + First of all, why did I review this? I implemented parsing a specific HTTP + auth header credential, which is non-trivial to do with just string splitting. + I had gathered some experience with `yap` during last years Advent of Code, + and I'm relatively happy with the implementation now. The only thing I was + missing is something like `Tokens::optional` that was based on `Result` + instead of `Option`. I'll make sure to contribute that later though. + + As for problems I've found while reviewing this crate: + + - There's one sound use of unsafe, that could probably be avoided, just for + good measure. It's about going from a `str` to the next `char`. It + increases a counter by one, and then by another one until it's at a char + boundary. This is explicitly checked before it then does + `str::get_unchecked`, which is the unsafe call. It seems sound to me, but + without tests this isn't a great look. + + - The formatting is a bit off in some places. A few extra spaces, nothing + big, just made reviewing a bit more of a chore. + + - The `logos` dev-dependency doesn't seem to be used anywhere. As `logos` is + quite wide-spread, it being included in yap is not much of a supply chain + safety loss, but when it's not used it should probably be dropped anyway. + + Things I specifically liked: + + - The documentation is amazing. Every single thing one could hope for has a + doc comment, and the documentation really goes above and beyond to + demonstrate how yap can be used. + + - Leveraging iterators was a very good choice. Using `yap` feels super + natural, and even without any prior parser-combinator experience, I was + able to quickly get started. Not using regular expressions and string + splitting feels really good. + + In general, I'm extremely happy with yap, and even happier now that I've done + my due diligence with regards to supply chain security. Thanks to the authors. +----- SIGN CREV PROOF ----- +wWTVLbr4D3hgMVVz8g7Q5GQdiSXO-_FlbVdQ0rWJ7jMNValXnZGLnDNPiM5gZFx9-JCw95pmZOpRaHMjBlrnDw +----- END CREV PROOF ----- +