Bug 5278

Summary: Review Request: mpc-qt - A clone of Media Player Classic reimplemented in Qt
Product: Package Reviews Reporter: leigh scott <leigh123linux>
Component: Review RequestAssignee: Vasiliy Glazov <vascom2>
Status: RESOLVED FIXED    
Severity: enhancement CC: akarshan.biswas, rpmfusion-package-review, vascom2
Priority: P1 Flags: vascom2: fedora-review+
Version: Current   
Hardware: x86_64   
OS: GNU/Linux   
namespace: free

Description leigh scott 2019-06-01 10:33:32 CEST
SPEC: https://leigh123linux.fedorapeople.org/pub/review/mpc-qt/1/mpc-qt.spec
SRPM: https://leigh123linux.fedorapeople.org/pub/review/mpc-qt/1/mpc-qt-18.08-1.fc30.src.rpm

Description:

Media Player Classic Home Cinema (mpc-hc) is considered by many to be the 
quintessential media player for the Windows desktop. 
Media Player Classic Qute Theater (mpc-qt) aims to reproduce most of the 
interface and functionality of mpc-h.

Fas: leigh123linux
Comment 1 Vasiliy Glazov 2019-06-04 13:41:10 CEST
1. Change license to GPLv2+.

2. In %files section license file must be in
%license LICENSE
not in %doc.

Are you shure that you want add to repo project with last commit almost a year ago?
Comment 2 leigh scott 2019-06-04 14:13:05 CEST
(In reply to Vasiliy Glazov from comment #1)
> 1. Change license to GPLv2+.
> 
> 2. In %files section license file must be in
> %license LICENSE
> not in %doc.
>

1 and 2 have been fixed

> Are you shure that you want add to repo project with last commit almost a
> year ago?

Yes
Comment 3 Vasiliy Glazov 2019-06-04 14:40:36 CEST
Approved.
Comment 4 Akarshan Biswas 2019-06-04 14:46:25 CEST
I might be wrong but I find no mention appdata either in the spec file or in the source rpm. Isn't supposed to add an appdata file to for a GUI application make it discover able in GUI software managers as a rule?


Cheers,
Akarshan Biswas
Comment 5 Vasiliy Glazov 2019-06-04 14:54:15 CEST
https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/

If a package contains a GUI application, then it SHOULD install a .appdata.xml file into %{_metainfodir}.
Comment 6 Akarshan Biswas 2019-06-04 15:08:42 CEST
(In reply to Vasiliy Glazov from comment #5)
> https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/
> 
> If a package contains a GUI application, then it SHOULD install a
> .appdata.xml file into %{_metainfodir}.

Exactly, I don't see any reference of %{metainfodir} in the %file section. Though I admit I haven't checked the source archive. An appdata file required before this package can be approved because mpc-qt is one of the GUI frontend for mpv written in Qt similar to gnome-mpv.



Cheers,
Akarshan Biswas
Pronouns: He/Him/His
TZ=Asia/Kolkata
Comment 7 Vasiliy Glazov 2019-06-04 15:13:13 CEST
It is not mandatory.
Comment 8 Vasiliy Glazov 2019-06-04 15:14:30 CEST
As I see mpc-qt - not frontend to mpv like smplayer or gnome-mpv. Ut just use mpv-libs to play video.
Comment 9 Akarshan Biswas 2019-06-04 15:18:46 CEST
(In reply to Vasiliy Glazov from comment #8)
> As I see mpc-qt - not frontend to mpv like smplayer or gnome-mpv. Ut just
> use mpv-libs to play video.

It's a GUI frontend for mpv. Qt is an alternative toolkit like GTK. Please see: https://github.com/mpv-player/mpv/wiki/Applications-using-mpv
Comment 10 leigh scott 2019-06-04 15:45:29 CEST
(In reply to Vasiliy Glazov from comment #3)
> Approved.

Thanks for the review can you set the + flag so I can process the request please?

Fedora-review flag not approved
Comment 11 leigh scott 2019-06-04 15:48:47 CEST
@vascom2 When your ready for the gnome-mpv rereview needed for the rename

https://github.com/celluloid-player/celluloid

Feel free to assign it to me.
Comment 12 leigh scott 2019-06-04 15:50:58 CEST
(In reply to leigh scott from comment #10)
> (In reply to Vasiliy Glazov from comment #3)
> > Approved.
> 
> Thanks for the review can you set the + flag so I can process the request
> please?
> 
> Fedora-review flag not approved

Can you set it again as I have clubbed the + with my last response :-(
Comment 13 Nicolas Chauvet 2019-08-26 13:22:25 CEST
Seems imported.