Spec URL: https://download.copr.fedorainfracloud.org/results/jstanek/rust-clang-ast/fedora-rawhide-x86_64/02285123-rust-clang-ast/rust-clang-ast.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/jstanek/rust-clang-ast/fedora-rawhide-x86_64/02285123-rust-clang-ast/rust-clang-ast-0.1.6-1.fc35.src.rpm Description: Data structures for processing Clang's `-ast-dump=json` format. Transitive build-time dependency of the newsboat package. Fedora Account System Username: jstanek
I'll take on this review. If you want to review another Rust package in turn: https://bugzilla.redhat.com/show_bug.cgi?id=1973218
Minor issues: 0) remove markdown markup from Summary Please remove markdown markup characters (`) from the Summary tag. 1) "# check has missing dependencies" Please list which dependencies are missing. 2) %description Looks like you changed "Clang's" to "Clang" in the generated description. Why? 3) %license in %files To be consistent with ~all~ other Rust packages (and to make version rebases easier for me), change to this please: %files devel %license LICENSE-APACHE LICENSE-MIT %doc README.md 4) changelog date format Please remove the time and timezone from the date in the changelog entry. This broken format is only used by rust2rpm any longer and I hope to be able to remove it there with the next version as well. --- All those are very minor or cosmetic issues. Other than those: Package was generated with rust2rpm, simplifying the review process. - package conforms to Rust packaging Guidelines ✅ - Latest version is packaged ✅ - License correct, acceptable, and LICENSE files shipped with %license ✅ - package builds and installs without errors in mock / rawhide ✅ Package APPROVED. Please make the small adaptations listed above before importing the package to Fedora.
(In reply to Fabio Valentini from comment #2) > 0) remove markdown markup from Summary ✔ > 1) "# check has missing dependencies" listed clang-ast-test-suite ✔ > 2) %description > Looks like you changed "Clang's" to "Clang" in the generated description. > Why? It messes my syntax higlighting in vim, and I forgot to undo the change once I was done with the spec. Apostrophe added back 🙂 > 3) %license in %files Changed as requested. ✔ > 4) changelog date format Time and timezone removed. ✔ --- Thanks for the review! The one in kind should be approved now.
Thanks! Don't forget to request the f34 branch as well: fedpkg request-branch --repo rust-clang-ast f34 (You can run this even if the repo does not exist yet, since those tickets are processed chronologically.)
(In reply to Fabio Valentini from comment #4) > (You can run this even if the repo does not exist yet, since those tickets > are processed chronologically.) That is good to know – the guidelines state this should be run *after* the repo is created. This is much more convenient 😀 https://pagure.io/releng/fedora-scm-requests/issue/35189 ← the repo https://pagure.io/releng/fedora-scm-requests/issue/35192 ← the branch
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-clang-ast
Successfully built in rawhide: https://bodhi.fedoraproject.org/updates/FEDORA-2021-ff9c7e3722