Bug 2335839 - Review Request: rust-async-std-resolver - A library implementing the DNS resolver using the Hickory DNS Resolver library
Summary: Review Request: rust-async-std-resolver - A library implementing the DNS reso...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL: https://crates.io/crates/async-std-re...
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2025-01-06 15:33 UTC by Umut Demir
Modified: 2025-01-28 20:02 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2025-01-28 20:02:27 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 8576864 to 8578616 (1.32 KB, patch)
2025-01-27 17:38 UTC, Fedora Review Service
no flags Details | Diff

Description Umut Demir 2025-01-06 15:33:55 UTC
Spec URL: https://copr-dist-git.fedorainfracloud.org/cgit/umutd3401/test-builds/rust-async-std-resolver.git/plain/rust-async-std-resolver.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/umutd3401/test-builds/srpm-builds/08475181/rust-async-std-resolver-0.24.2-1.src.rpm
Description: This Resolver library uses the hickory-proto library to perform all DNS queries. The Resolver is intended to be a high-level library for any DNS record resolution.
Fedora Account System Username: umutd3401

Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=127605692

Comment 1 Fedora Review Service 2025-01-06 15:41:53 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8475342
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2335839-rust-async-std-resolver/fedora-rawhide-x86_64/08475342-rust-async-std-resolver/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 2 Fabio Valentini 2025-01-26 21:19:08 UTC
Can you reupload the spec / SRPM files? They're giving HTTP 404 errors now.

Comment 4 Fedora Review Service 2025-01-27 05:21:50 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8576864
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2335839-rust-async-std-resolver/fedora-rawhide-x86_64/08576864-rust-async-std-resolver/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 5 Fabio Valentini 2025-01-27 10:58:54 UTC
Thanks!

There are some differences from the spec file generated by rust2rpm that I don't understand:

-Name:           rust-async-std-resolver
+Name:           rust-%{crate}

Why change this? While this is equivalent, I wouldn't recommend changing anything that rust2rpm generates, just because you'd need to re-do those changes on any version updates when re-generating the spec file.

-Summary:        Hickory DNS is a safe and secure DNS library, for async-std
+Summary:        A library implementing the DNS resolver using the Hickory DNS Resolver library

The second one is, IMO, worse, though both are not good.
"A library implementing (...)" is more or less only fluff - Summary tags should not start with "A" or "An", and it's clear that this is a library from the package type.

I would suggest using "Summary: DNS resolver implementation based on async-std and Hickory DNS" or something like that.

You can override this with a rust2rpm.toml setting so you don't have to re-apply this change with every update.

Similarly, for the %description - though why change it at all? The generated one seems fine.

-%bcond check 1
+%bcond_with check

If this spec was generated by rust2rpm v27, why is there the old bcond syntax? Just swap 1 for 0 to disable tests, no need to go back to ancient times :)

Comment 6 Umut Demir 2025-01-27 17:30:45 UTC
Thanks for the review!

(In reply to Fabio Valentini from comment #5)
> Why change this? While this is equivalent, I wouldn't recommend changing
> anything that rust2rpm generates, just because you'd need to re-do those
> changes on any version updates when re-generating the spec file.

> If this spec was generated by rust2rpm v27, why is there the old bcond
> syntax? Just swap 1 for 0 to disable tests, no need to go back to ancient
> times :)

I will be careful not to change anything unless necessary, thank you.

> The second one is, IMO, worse, though both are not good.
> "A library implementing (...)" is more or less only fluff - Summary tags
> should not start with "A" or "An", and it's clear that this is a library
> from the package type.
> 
> I would suggest using "Summary: DNS resolver implementation based on
> async-std and Hickory DNS" or something like that.

Done. The packaging guidelines show an example that starts with an "A" as a good practice, so I thought it was acceptable.

> Similarly, for the %description - though why change it at all? The generated
> one seems fine.

Some other crates related to Hickory DNS also start like that. I just didn't want to make them all same and wanted to provide information about the package itself more. It is not necessary to do this, of course. Changed it.

Spec URL: https://download.copr.fedorainfracloud.org/results/umutd3401/test-builds/fedora-rawhide-x86_64/08578602-rust-async-std-resolver/rust-async-std-resolver.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/umutd3401/test-builds/fedora-rawhide-x86_64/08578602-rust-async-std-resolver/rust-async-std-resolver-0.24.2-1.fc42.src.rpm

Comment 7 Fedora Review Service 2025-01-27 17:38:22 UTC
Created attachment 2074046 [details]
The .spec file difference from Copr build 8576864 to 8578616

Comment 8 Fedora Review Service 2025-01-27 17:38:24 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8578616
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2335839-rust-async-std-resolver/fedora-rawhide-x86_64/08578616-rust-async-std-resolver/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 9 Fabio Valentini 2025-01-27 21:04:49 UTC
> Done. The packaging guidelines show an example that starts with an "A" as a good practice, so I thought it was acceptable.

Where? 😬 That should be fixed. I think there are even lints somewhere that check that Summary tag doesn't start with "A", "An", or "The" :D

Comment 10 Fabio Valentini 2025-01-27 21:12:19 UTC
And ... the package looks good to me now, thanks!

===

Package was generated with rust2rpm, simplifying the review.

✅ package contains only permissible content
✅ package builds and installs without errors on rawhide
🫤 test suite is run and all unit tests pass (tests skipped with explanation)
✅ latest version of the crate is packaged
✅ license matches upstream specification and is acceptable for Fedora
✅ license files are included with %license in %files
✅ package complies with Rust Packaging Guidelines

Package APPROVED.

===

Recommended post-import rust-sig tasks:

- set up package on release-monitoring.org:
  project: $crate
  homepage: https://crates.io/crates/$crate
  backend: crates.io
  version scheme: semantic
  version filter (*NOT* pre-release filter): alpha;beta;rc;pre
  distro: Fedora
  Package: rust-$crate

- add @rust-sig with "commit" access as package co-maintainer
  (should happen automatically)

- set bugzilla assignee overrides to @rust-sig (optional)

- track package in koschei for all built branches
  (should happen automatically once rust-sig is co-maintainer)

Comment 11 Umut Demir 2025-01-28 05:21:12 UTC
(In reply to Fabio Valentini from comment #9)
> Where? 😬 That should be fixed. I think there are even lints somewhere that
> check that Summary tag doesn't start with "A", "An", or "The" :D

https://docs.fedoraproject.org/en-US/packaging-guidelines/#_trademarks_in_summary_or_description

Also 'fedpkg lint' doesn't give me any errors or warnings about it.

Comment 12 Fedora Admin user for bugzilla script actions 2025-01-28 18:46:00 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-async-std-resolver

Comment 13 Fedora Update System 2025-01-28 19:59:47 UTC
FEDORA-2025-c69c5321bd (rust-async-std-resolver-0.24.2-1.fc42) has been submitted as an update to Fedora 42.
https://bodhi.fedoraproject.org/updates/FEDORA-2025-c69c5321bd

Comment 14 Fedora Update System 2025-01-28 20:02:27 UTC
FEDORA-2025-c69c5321bd (rust-async-std-resolver-0.24.2-1.fc42) has been pushed to the Fedora 42 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.