Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1974377
Summary: | Review Request: rust-clang-ast - Data structures for processing Clang's `-ast-dump=json` format | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Jan Staněk <jstanek> |
Component: | Package Review | Assignee: | Fabio Valentini <decathorpe> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | decathorpe, package-review |
Target Milestone: | --- | Flags: | decathorpe:
fedora-review+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | If docs needed, set a value | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2021-06-23 13:02:04 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 1974172, 1974529 |
Description
Jan Staněk
2021-06-21 13:49:54 UTC
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 |