Bug 1087536
Summary: | Review Request: perl-HTML-FormFu-MultiForm - Handle multi-page/stage forms | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Petr Pisar <ppisar> |
Component: | Package Review | Assignee: | Ralf Corsepius <rc040203> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | package-review, rc040203 |
Target Milestone: | --- | Flags: | rc040203:
fedora-review+
petersen: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | perl-HTML-FormFu-MultiForm-1.00-1.fc21 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2014-04-17 07:22:39 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: | 1085432 |
Description
Petr Pisar
2014-04-14 14:47:35 UTC
Package basically looks fine to me, except that it currently fails to build in mock: ... Error: No Package found for perl(Test::Aggregate::Nested) >= 0.371 ... I noticed perl-Test-Aggregate-0.371 already is in koji, but apparently it hasn't reached the mirrors, yet. I'll continue this review once it has. APPROVED 2 remarks on issues, you probably are aware about: - The package treats Crypt::DES as required dependency, while it actually doesn't use it (You seem to have commented out BR: perl(Crypt::DES) because of this) I.e. this package only builds by random coincidence, because another package (perl-HTML-FormFu) indirectly pulls in perl-Crypt-DES. If perl-HTML-FormFu didn't do so, building this package would fail. I'd recommend to patch out Crypt::DES from the source code. - I am not happy with "PERL_HASH_SEED=0 make test", because it's not clear to me whether this is just an issue with the test-suite or whether this is a defect of this package in general. (In reply to Ralf Corsepius from comment #2) > - The package treats Crypt::DES as required dependency, while it actually > doesn't use it (You seem to have commented out BR: perl(Crypt::DES) because > of this) > > I.e. this package only builds by random coincidence, because another package > (perl-HTML-FormFu) indirectly pulls in perl-Crypt-DES. If perl-HTML-FormFu > didn't do so, building this package would fail. > > I'd recommend to patch out Crypt::DES from the source code. > That's interesting problem. The Crypt::CBC calls from this package do not request Crypt::DES anywhere in the code. None CBC::Crypt->new() passes -cipher argument. The code relays on CBC::Crypt default algorithm which is CBC::DES. However perl-Crypt-CBC does require Crypt::DES. I would rather see perl-Crypt-CBC run-requiring Crypt::DES to conform to Crypt::CBC documentation. Unfortunately the perl-Crypt-CBC is in dependency cycle with perl-Crypt-DES and Paul decided to make the dependency on Crypt::DES optional. So I will add the dependency on Crypt::DES into perl-HTML-FormFu-MultiForm to follow the upstream path. New Package SCM Request ======================= Package Name: perl-HTML-FormFu-MultiForm Short Description: Handle multi-page/stage forms Owners: ppisar jplesnik psabata Branches: InitialCC: perl-sig Git done (by process-git-requests). Thank you for the review and the repository. |