Bug 1632439 - Review Request: perl-Mojolicious-Plugin-OAuth2 - Mojolicious plugin for OAuth2 authentication
Summary: Review Request: perl-Mojolicious-Plugin-OAuth2 - Mojolicious plugin for OAuth...
Keywords:
Status: CLOSED WORKSFORME
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: 28
Hardware: noarch
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Jitka Plesnikova
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/marcusramberg/Mojo...
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-09-24 19:40 UTC by Mike Oliver
Modified: 2018-10-23 12:23 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2018-10-23 12:23:38 UTC
Type: ---
Embargoed:
jplesnik: fedora-review+


Attachments (Terms of Use)

Description Mike Oliver 2018-09-24 19:40:22 UTC
Spec URL: https://mklvr.fedorapeople.org/fedora-packages/perl-Mojolicious-Plugin-OAuth2/perl-Mojolicious-Plugin-OAuth2.spec
SRPM URL: https://mklvr.fedorapeople.org/fedora-packages/perl-Mojolicious-Plugin-OAuth2/perl-Mojolicious-Plugin-OAuth2-1.57-1.fc28.src.rpm
Description: This Mojolicious plugin allows you to easily authenticate against a OAuth2 provider. It includes configurations for a few popular providers, but you can add your own easily as well.
Fedora Account System Username: mklvr

Comment 1 Jitka Plesnikova 2018-09-25 08:18:15 UTC
Source file is ok
Summary is ok
License is ok
Description is ok
All tests passed

URL and Source0 are ok
TODO: Is there any special reason why you use url
      https://github.com/marcusramberg/Mojolicious-Plugin-OAuth2
      instead of metacpan.org as most of Perl packages?
      Please consider the changing to
      Source0: https://metacpan.org/release/Mojolicious-Plugin-OAuth2
      URL: https://cpan.metacpan.org/authors/id/M/MR/MRAMBERG/Mojolicious-Plugin-OAuth2-%{version}.tar.gz

BuildRequires 
FIX: Please add build-requires
     - %{__make}
     - perl(Carp) - lib/Mojolicious/Plugin/OAuth2.pm:6
     - perl(File::Find) - t/00-basic.t:2
     - perl(Mojo::Promise) - lib/Mojolicious/Plugin/OAuth2.pm:4
     - perl(Mojo::UserAgent) - lib/Mojolicious/Plugin/OAuth2.pm:5
     - perl(Mojolicious::Plugin) - lib/Mojolicious/Plugin/OAuth2.pm:2
FIX: Please remove build-requires perl(Scalar::Util) and perl(Test::Output).
     I did not find any using of them.

$ rpm -qp --requires perl-Mojolicious-Plugin-OAuth2-1.57-1.fc30.noarch.rpm | sort | uniq -c
      1 perl(Carp)
      1 perl(IO::Socket::SSL)
      1 perl(Mojo::Base)
      1 perl(Mojolicious) >= 7.53
      1 perl(Mojo::Promise)
      1 perl(Mojo::UserAgent)
      1 perl(strict)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
Binary requires 
FIX: Please add run requires
     - perl(Mojolicious::Plugin) - lib/Mojolicious/Plugin/OAuth2.pm:2

$ rpm -qp --provides perl-Mojolicious-Plugin-OAuth2-1.57-1.fc30.noarch.rpm | sort | uniq -c
      1 perl(Mojolicious::Plugin::OAuth2) = 1.57
      1 perl-Mojolicious-Plugin-OAuth2 = 1.57-1.fc30
Binary provides are Ok.

$ rpmlint ./perl-Mojolicious-Plugin-OAuth2*
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
Rpmlint is ok

Please correct all 'FIX' issues and consider fixing 'TODO' items and
provide new spec file.

Package is NOT approved.

Comment 2 Mike Oliver 2018-09-25 14:03:43 UTC
Spec URL: https://mklvr.fedorapeople.org/fedora-packages/perl-Mojolicious-Plugin-OAuth2/perl-Mojolicious-Plugin-OAuth2.spec
SRPM URL: https://mklvr.fedorapeople.org/fedora-packages/perl-Mojolicious-Plugin-OAuth2/perl-Mojolicious-Plugin-OAuth2-1.57-1.fc28.src.rpm
Description: This Mojolicious plugin allows you to easily authenticate against a OAuth2 provider. It includes configurations for a few popular providers, but you can add your own easily as well.
Fedora Account System Username: mklvr

Requested changes have been made. Spec and SRPM have been updated.

As for the TODO, at the time of package creation, I believe release 1.57 was only available on GitHub. It's available on metacpan.org now, so I've made the requested changes. 

Thanks for the review!

Comment 3 Jitka Plesnikova 2018-09-25 14:32:35 UTC
> URL and Source0 are ok
> TODO: Is there any special reason why you use url
>       https://github.com/marcusramberg/Mojolicious-Plugin-OAuth2
>       instead of metacpan.org as most of Perl packages?
>       Please consider the changing to
>       Source0: https://metacpan.org/release/Mojolicious-Plugin-OAuth2
>       URL:
> https://cpan.metacpan.org/authors/id/M/MR/MRAMBERG/Mojolicious-Plugin-OAuth2-
> %{version}.tar.gz
-URL:       https://github.com/marcusramberg/Mojolicious-Plugin-OAuth2
-Source0:   https://github.com/marcusramberg/Mojolicious-Plugin-OAuth2/archive/%{version}/Mojolicious-Plugin-OAuth2-%{version}.tar.gz
+URL:       https://metacpan.org/release/Mojolicious-Plugin-OAuth2
+Source0:   https://cpan.metacpan.org/authors/id/M/MR/MRAMBERG/Mojolicious-Plugin-OAuth2-%{version}.tar.gz
 
Fixed

> BuildRequires 
> FIX: Please add build-requires
>      - %{__make}
>      - perl(Carp) - lib/Mojolicious/Plugin/OAuth2.pm:6
>      - perl(File::Find) - t/00-basic.t:2
>      - perl(Mojo::Promise) - lib/Mojolicious/Plugin/OAuth2.pm:4
>      - perl(Mojo::UserAgent) - lib/Mojolicious/Plugin/OAuth2.pm:5
>      - perl(Mojolicious::Plugin) - lib/Mojolicious/Plugin/OAuth2.pm:2
+BuildRequires:  perl(Carp)
+BuildRequires:  perl(File::Find)
+BuildRequires:  perl(Mojo::Promise)
+BuildRequires:  perl(Mojo::UserAgent)
+BuildRequires:  perl(Mojolicious::Plugin)
Fixed

> FIX: Please remove build-requires perl(Scalar::Util) and perl(Test::Output).
>      I did not find any using of them.
-BuildRequires:  perl(Scalar::Util)
-BuildRequires:  perl(Test::Output) >= 1
Fixed

> FIX: Please add run requires
>      - perl(Mojolicious::Plugin) - lib/Mojolicious/Plugin/OAuth2.pm:2
+Requires:  perl(Mojolicious::Plugin)
Fixed
 
The package look good now.
Approved

Comment 4 Gwyn Ciesla 2018-09-26 18:26:55 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/perl-Mojolicious-Plugin-OAuth2

Comment 5 Jitka Plesnikova 2018-10-23 11:01:16 UTC
Could it be closed?

Comment 6 Mike Oliver 2018-10-23 12:23:38 UTC
Yes, this can be closed. Thanks!


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