SPEC: https://pbrobinson.fedorapeople.org/rust-ciborium.spec SRPM: https://pbrobinson.fedorapeople.org/rust-ciborium-0.2.0-1.fc36.src.rpm Descrption: Serde implementation of CBOR using ciborium-basic FAS: pbrobinson
Two minor issues: - Why are you disabling the test suite? Either enable it, or add a short comment above "%bcond_with check" to explain **why** it's disabled. - Are you not using rpmautospec for this package on purpose? Both ciborium-io and ciborium-ll use it, it strikes me as odd that this package doesn't.
> - Are you not using rpmautospec for this package on purpose? > Both ciborium-io and ciborium-ll use it, it strikes me as odd that this > package doesn't. It was literally done with rpm2rust, maybe I did it previously with an older version and didn't regenerate it.
> It was literally done with rpm2rust, maybe I did it previously with an older version and didn't regenerate it. That's unlikely, since the rest of the .spec file uses stuff that has only been there with rust2rpm 21. > - Why are you disabling the test suite? > Either enable it, or add a short comment above "%bcond_with check" to explain **why** it's disabled. This remains unresolved.
> This remains unresolved. If it's about the missing "rstest" dev-dependency for the tests, just adding this above "%bcond_with check" would be enough: # * missing dev-dependency: rstest
(In reply to Fabio Valentini from comment #4) > > This remains unresolved. > > If it's about the missing "rstest" dev-dependency for the tests, just adding > this above "%bcond_with check" would be enough: > > # * missing dev-dependency: rstest Why isn't that added by rust2rpm?
> Why isn't that added by rust2rpm? Uh, how? Not sure how that would even work. Should rust2rpm it check for all dependencies whether they're available from Fedora repos? That would make it very slow. And even that would break down as soon as you actually would want to update an outdated dependency, or package a missing one.
(In reply to Fabio Valentini from comment #6) > > Why isn't that added by rust2rpm? > > Uh, how? Not sure how that would even work. > Should rust2rpm it check for all dependencies whether they're available from > Fedora repos? That would make it very slow. > And even that would break down as soon as you actually would want to update > an outdated dependency, or package a missing one. Sorry, you misinterpreted my misunderstanding. I thought it was something that was missing in the actual spec file, not a missing dependency.
It looks like there's mutual misunderstanding ... What you need to do is provide a short explanatory comment *why* you have *already manually edited* the .spec file that was generated by rust2rpm (to disable dev-dependencies and not run the test suite). You replaced the line %bcond_without check with just %bcond_with check but it should have been replaced with *two lines* like these # * missing dev-dependency: rstest %bcond_with check because the reason for disabling a package's test suite needs to be documented in the .spec file.
> What you need to do is provide a short explanatory comment *why* you have > *already manually edited* the .spec file that was generated by rust2rpm (to > disable dev-dependencies and not run the test suite). Yes, I normally do, clearly something slipped through the cracks, it wasn't malicious or intentional, it was probably an oversight from testing when tired/stressed, I feel I have enough credit with in the community to be given the benefit of the doubt here. I don't see why a simple "this needs a comment as to why you're disabling tests, please ensure you do this on commit" wouldn't have been enough. Spec/srpm now updated.
I'm sorry if my last comment was unnecessarily explicit. It seemed to me you had misunderstood my previous comments, but apparently that was not the case. > I don't see why a simple "this needs a comment as to why you're disabling tests, please ensure you do this on commit" wouldn't have been enough. Because I've had bad experiences with doing this. Some people just ignored (or didn't see) those caveats, they only saw "fedora-review+" and went on importing the package without applying the necessary changes first. Either way, the package looks good to me now, thank you. === Package was generated with rust2rpm, simplifying the review. - package builds and installs without errors on rawhide - test suite is run and all unit tests pass - latest version of the crate is packaged - license matches upstream specification (ASL 2.0) and is acceptable for Fedora - license file is included with %license in %files - package complies with Rust Packaging Guidelines Package APPROVED. === Recommended post-import rust-sig tasks: - add @rust-sig with "commit" access as package co-maintainer - set bugzilla assignee overrides to @rust-sig (optional) - set up package on release-monitoring.org: project: $crate homepage: https://crates.io/crates/$crate backend: crates.io version scheme: semantic version filter: alpha;beta;rc;pre distro: Fedora Package: rust-$crate - track package in koschei for all built branches
Meh, copypasta. > - test suite is run and all unit tests pass This should have been: - test suite is disabled; explanatory comment is present
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-ciborium