From ee6ef0f2968ea678dfc14c24a3f2c1b32ea14ec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Christian=20Gr=C3=BCnhage?= Date: Mon, 27 Jun 2022 23:46:01 +0200 Subject: [PATCH] Overwrite 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 1f0c6a4..a7f0e05 100644 --- a/YdnEoYtqvbBGv0hhENLDUYmc3tNfm5V5NIG5hCovHyM/reviews/2022-06-package-0Oh57g.proof.crev +++ b/YdnEoYtqvbBGv0hhENLDUYmc3tNfm5V5NIG5hCovHyM/reviews/2022-06-package-0Oh57g.proof.crev @@ -89,3 +89,69 @@ comment: |- wWTVLbr4D3hgMVVz8g7Q5GQdiSXO-_FlbVdQ0rWJ7jMNValXnZGLnDNPiM5gZFx9-JCw95pmZOpRaHMjBlrnDw ----- END CREV PROOF ----- +----- BEGIN CREV PROOF ----- +kind: package review +version: -1 +date: "2022-06-27T23:46:01.955622651+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. + + - A previous version of this review claimed that the `logos` dev-dependency + wasn't used anywhere. That's true for the releases on crates.io, but the + repository contains examples, where `logos` is actually used. + + 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 ----- +Zjv7bZRcXGnY0bHWTqtrHnO02n07ztgRgEEG8gjk7RYtt2KZC-oYNzRVHGYwAvicP31CfXaxCT38RNl5Z4WIAQ +----- END CREV PROOF ----- +