[Bug 4028] Review request: fs-uae-launcher - Graphical configuration frontend and launcher for FS-UAE

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=4028

Andrea Musuruane  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #10 from Andrea Musuruane  ---
Imported and built!

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
rpmfusion-developers mailing list -- rpmfusion-developers@lists.rpmfusion.org
To unsubscribe send an email to rpmfusion-developers-le...@lists.rpmfusion.org

[Bug 4028] Review request: fs-uae-launcher - Graphical configuration frontend and launcher for FS-UAE

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=4028

--- Comment #9 from Andrea Musuruane  ---
Thanks for the review!

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
rpmfusion-developers mailing list -- rpmfusion-developers@lists.rpmfusion.org
To unsubscribe send an email to rpmfusion-developers-le...@lists.rpmfusion.org

[Bug 4028] Review request: fs-uae-launcher - Graphical configuration frontend and launcher for FS-UAE

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=4028

Ben Rosser  changed:

   What|Removed |Added

 Blocks|3   |
  Flags|fedora-review?  |fedora-review+

--- Comment #8 from Ben Rosser  ---
Everything now looks fine, so package APPROVED.


Referenced Bugs:

https://bugzilla.rpmfusion.org/show_bug.cgi?id=3
[Bug 3] Tracker: Packages under review.
-- 
You are receiving this mail because:
You are on the CC list for the bug.___
rpmfusion-developers mailing list -- rpmfusion-developers@lists.rpmfusion.org
To unsubscribe send an email to rpmfusion-developers-le...@lists.rpmfusion.org

[Bug 4028] Review request: fs-uae-launcher - Graphical configuration frontend and launcher for FS-UAE

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=4028

--- Comment #7 from Andrea Musuruane  ---
https://www.dropbox.com/s/1x6f9bgktd1hvn7/fs-uae-launcher.spec?dl=0
https://www.dropbox.com/s/hwlu8wqfcv124zu/fs-uae-launcher-2.8.3-4.fc26.src.rpm?dl=0

Changelog:
- Added a virtual provide to note oyoyo is bundled
- Amended License tag


(In reply to Ben Rosser from comment #6)
> Don't forget to add a bundled provides on python-oyoyo, too, even though it
> is not packaged in Fedora. Otherwise the package looks good now, but...

Added.

> > Regarding the license, please remember the license tag is not a list of 
> > licenses found in the source archive but it refers to the licenses of the 
> > contents of the *binary RPM*:
> 
> While that's true, the reason I commented is because the package isn't
> compiling GPL and MIT sources into a single binary, it's installing the
> separate, MIT licensed Python *source* files into /usr/share.
> 
> > $ rpmls ./fs-uae-launcher-2.8.3-2.fc28.noarch.rpm
> > ...
> > -rw-r--r--  /usr/share/fs-uae-launcher/oyoyo/client.py
> > -rw-r--r--  /usr/share/fs-uae-launcher/oyoyo/cmdhandler.py
> > -rw-r--r--  /usr/share/fs-uae-launcher/oyoyo/helpers.py
> > -rw-r--r--  /usr/share/fs-uae-launcher/oyoyo/ircevents.py
> > -rw-r--r--  /usr/share/fs-uae-launcher/oyoyo/parse.py
> 
> And the guidelines have this to say about that:
> 
> > If your package contains files which are under multiple, distinct, and 
> > independent licenses, then the spec must reflect this by using "and" as a 
> > separator.
> 
> https://fedoraproject.org/wiki/Packaging:
> LicensingGuidelines#Multiple_Licensing_Scenarios
> 
> That being said, I'm personally inclined to agree with you that, since the
> licenses are compatible, it's not necessary to include both here. So I'm
> inclined to let this slide.

You are formally correct. But that is a private copy only to be used by
fs-uae-launcher. No other package should ever Require: it.

Anyway, even if I disagree, I amended the License field.

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
rpmfusion-developers mailing list -- rpmfusion-developers@lists.rpmfusion.org
To unsubscribe send an email to rpmfusion-developers-le...@lists.rpmfusion.org

[Bug 4028] Review request: fs-uae-launcher - Graphical configuration frontend and launcher for FS-UAE

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=4028

--- Comment #6 from Ben Rosser  ---
Don't forget to add a bundled provides on python-oyoyo, too, even though it is
not packaged in Fedora. Otherwise the package looks good now, but...

> Regarding the license, please remember the license tag is not a list of 
> licenses found in the source archive but it refers to the licenses of the 
> contents of the *binary RPM*:

While that's true, the reason I commented is because the package isn't
compiling GPL and MIT sources into a single binary, it's installing the
separate, MIT licensed Python *source* files into /usr/share.

> $ rpmls ./fs-uae-launcher-2.8.3-2.fc28.noarch.rpm
> ...
> -rw-r--r--  /usr/share/fs-uae-launcher/oyoyo/client.py
> -rw-r--r--  /usr/share/fs-uae-launcher/oyoyo/cmdhandler.py
> -rw-r--r--  /usr/share/fs-uae-launcher/oyoyo/helpers.py
> -rw-r--r--  /usr/share/fs-uae-launcher/oyoyo/ircevents.py
> -rw-r--r--  /usr/share/fs-uae-launcher/oyoyo/parse.py

And the guidelines have this to say about that:

> If your package contains files which are under multiple, distinct, and 
> independent licenses, then the spec must reflect this by using "and" as a 
> separator.

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios

That being said, I'm personally inclined to agree with you that, since the
licenses are compatible, it's not necessary to include both here. So I'm
inclined to let this slide.

> Yes, it is necessary.

I see! That makes sense; I was unaware of this. (I forgot about the Python
guideline appendix...).

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
rpmfusion-developers mailing list -- rpmfusion-developers@lists.rpmfusion.org
To unsubscribe send an email to rpmfusion-developers-le...@lists.rpmfusion.org

[Bug 4028] Review request: fs-uae-launcher - Graphical configuration frontend and launcher for FS-UAE

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=4028

--- Comment #5 from Andrea Musuruane  ---
https://www.dropbox.com/s/cuwziwqwzg59bvo/fs-uae-launcher.spec?dl=0
https://www.dropbox.com/s/epardoy194b0aip/fs-uae-launcher-2.8.3-3.fc26.src.rpm?dl=0

Changelog:
- Added AppData file
- Removed six python library
- Unbundled font files

(In reply to Ben Rosser from comment #4)
> Really sorry for the delay here. I just attempted to run fedora-review by
> hand over the spec and SRPM, but it failed for some unclear reason (a python
> exception that I couldn't immediately parse).

No problem, really - I also miss some free time to continue to review your
package. I hope to resume in a couple of days.

> So looking by hand, I have a couple of initial comments:
> 
> * The package includes a graphical application; therefore you should write
> an appdata/appstream file. https://fedoraproject.org/wiki/Packaging:AppData

Fixed.

> * The results of "licensecheck" suggest there may be bundled libraries:
> 
> > .//oyoyo/__init__.py: MIT/X11 (BSD like)
> > .//oyoyo/client.py: MIT/X11 (BSD like)
> > .//oyoyo/cmdhandler.py: MIT/X11 (BSD like)
> > .//oyoyo/helpers.py: MIT/X11 (BSD like)
> > .//oyoyo/ircevents.py: MIT/X11 (BSD like)
> > .//oyoyo/parse.py: MIT/X11 (BSD like)
> > .//six/six.py: MIT/X11 (BSD like)
> > .//OpenGL/DLLS/gle_COPYING: Artistic GPL (with incorrect FSF address)
> 
> It looks like the only bundled bits under the OpenGL/DLLs directory are some
> libraries for Windows, which can safely be ignored. However, there is
> definitely a bundled copy of python-six. oyoyo appears to be
> https://pypi.python.org/pypi/oyoyo, I think, which doesn't appear to be
> packaged in Fedora. 
> 
> Please either unbundle or add a Bundled provides
> (https://fedoraproject.org/wiki/Bundled_Libraries?rd=Packaging:
> Bundled_Libraries), updating the License tag accordingly with "and MIT".

I unbundled six.

Regarding the license, please remember the license tag is not a list of
licenses found in the source archive but it refers to the licenses of the
contents of the *binary RPM*:
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License:_field

GPLv2+ and MIT are compatible licenses. Mixing them is fine and the resulting
license is GPLv2+.

You can also read more about this topic in this old thread:
https://lists.fedoraproject.org/archives/list/packag...@lists.fedoraproject.org/thread/QTXWYMQVWVGDGBF2DFQBDGWJNMEO4HHV/#HAVBN2535DPVZ47ID2J5ON65FXOFWS63

> > %global __python %{__python3}
> 
> Is this line really necessary? %__python isn't used anywhere in the spec. If
> it is not, I would suggest removing it; if it is, I would suggest including
> a comment as to its purpose. The Python guidelines seem to indicate you
> shouldn't need to use __python as that points at the "default" Python stack
> on the system: https://fedoraproject.org/wiki/Packaging:Python#Macros

Yes, it is necessary.

"Doing this is useful when you have a python3 application that's installing a
private module into its own directory. For instance, if the foobar application
installs a module for use only by the command line application in
%{_datadir}/foobar. Since these files are not in one of the python3 library
paths (ie. /usr/lib/python3.1) you have to override %{__python} to tell
brp-python-bytecompile to use the python3 interpreter for byte compiling."

https://fedoraproject.org/wiki/Packaging:Python_Appendix#Manual_byte_compilation

> > -rw-r--r--  
> > /usr/share/fs-uae-launcher/workspace/ui/data/NotoSans-Regular.ttf
> > -rw-r--r--  /usr/share/fs-uae-launcher/workspace/ui/data/Roboto-Regular.ttf
> 
> From the file list of the resulting package; these font files should
> probably be unbundled if possible. (They also appear to be provided by the
> google-noto* and google-roboto* font packages). If not possible, please
> instead check their licenses and add it to the License tag.
> 
> (See https://fedoraproject.org/wiki/Packaging:FontsPolicy for more).

Good catch. I unbundled these fonts and LiberationSans-Bold.ttf (which has the
same issue).

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
rpmfusion-developers mailing list -- rpmfusion-developers@lists.rpmfusion.org
To unsubscribe send an email to rpmfusion-developers-le...@lists.rpmfusion.org

[Bug 4028] Review request: fs-uae-launcher - Graphical configuration frontend and launcher for FS-UAE

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=4028

Ben Rosser  changed:

   What|Removed |Added

  Flags||fedora-review?

--- Comment #4 from Ben Rosser  ---
Really sorry for the delay here. I just attempted to run fedora-review by hand
over the spec and SRPM, but it failed for some unclear reason (a python
exception that I couldn't immediately parse).

So looking by hand, I have a couple of initial comments:

* The package includes a graphical application; therefore you should write an
appdata/appstream file. https://fedoraproject.org/wiki/Packaging:AppData

* The results of "licensecheck" suggest there may be bundled libraries:

> .//oyoyo/__init__.py: MIT/X11 (BSD like)
> .//oyoyo/client.py: MIT/X11 (BSD like)
> .//oyoyo/cmdhandler.py: MIT/X11 (BSD like)
> .//oyoyo/helpers.py: MIT/X11 (BSD like)
> .//oyoyo/ircevents.py: MIT/X11 (BSD like)
> .//oyoyo/parse.py: MIT/X11 (BSD like)
> .//six/six.py: MIT/X11 (BSD like)
> .//OpenGL/DLLS/gle_COPYING: Artistic GPL (with incorrect FSF address)

It looks like the only bundled bits under the OpenGL/DLLs directory are some
libraries for Windows, which can safely be ignored. However, there is
definitely a bundled copy of python-six. oyoyo appears to be
https://pypi.python.org/pypi/oyoyo, I think, which doesn't appear to be
packaged in Fedora. 

Please either unbundle or add a Bundled provides
(https://fedoraproject.org/wiki/Bundled_Libraries?rd=Packaging:Bundled_Libraries),
updating the License tag accordingly with "and MIT".

> %global __python %{__python3}

Is this line really necessary? %__python isn't used anywhere in the spec. If it
is not, I would suggest removing it; if it is, I would suggest including a
comment as to its purpose. The Python guidelines seem to indicate you shouldn't
need to use __python as that points at the "default" Python stack on the
system: https://fedoraproject.org/wiki/Packaging:Python#Macros

> -rw-r--r--  /usr/share/fs-uae-launcher/workspace/ui/data/NotoSans-Regular.ttf
> -rw-r--r--  /usr/share/fs-uae-launcher/workspace/ui/data/Roboto-Regular.ttf

>From the file list of the resulting package; these font files should probably
be unbundled if possible. (They also appear to be provided by the google-noto*
and google-roboto* font packages). If not possible, please instead check their
licenses and add it to the License tag.

(See https://fedoraproject.org/wiki/Packaging:FontsPolicy for more).

Otherwise, the package seems to look fine.

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
rpmfusion-developers mailing list -- rpmfusion-developers@lists.rpmfusion.org
To unsubscribe send an email to rpmfusion-developers-le...@lists.rpmfusion.org

[Bug 4028] Review request: fs-uae-launcher - Graphical configuration frontend and launcher for FS-UAE

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=4028

--- Comment #3 from Andrea Musuruane  ---
https://www.dropbox.com/s/vww2h3o3a7nopzq/fs-uae-launcher.spec?dl=0
https://www.dropbox.com/s/pkm3p66xypey3rm/fs-uae-launcher-2.8.3-2.fc26.src.rpm?dl=0

Changelog:
- Relaxed fs-uae requires

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
rpmfusion-developers mailing list -- rpmfusion-developers@lists.rpmfusion.org
To unsubscribe send an email to rpmfusion-developers-le...@lists.rpmfusion.org

[Bug 4028] Review request: fs-uae-launcher - Graphical configuration frontend and launcher for FS-UAE

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=4028

Ben Rosser  changed:

   What|Removed |Added

 CC||rosser@gmail.com
 Blocks|2   |3
   Assignee|rpmfusion-package-review@rp |rosser@gmail.com
   |mfusion.org |


Referenced Bugs:

https://bugzilla.rpmfusion.org/show_bug.cgi?id=2
[Bug 2] Tracker: New packages awaiting review
https://bugzilla.rpmfusion.org/show_bug.cgi?id=3
[Bug 3] Tracker: Packages under review.
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are the assignee for the bug.___
rpmfusion-developers mailing list -- rpmfusion-developers@lists.rpmfusion.org
To unsubscribe send an email to rpmfusion-developers-le...@lists.rpmfusion.org

[Bug 4028] Review request: fs-uae-launcher - Graphical configuration frontend and launcher for FS-UAE

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=4028
Bug 4028 depends on bug 4027, which changed state.

Bug 4027 Summary: Review request: fs-uae - Amiga emulator with on-screen GUI 
and online play support
https://bugzilla.rpmfusion.org/show_bug.cgi?id=4027

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are the assignee for the bug.___
rpmfusion-developers mailing list -- rpmfusion-developers@lists.rpmfusion.org
To unsubscribe send an email to rpmfusion-developers-le...@lists.rpmfusion.org

[Bug 4028] Review request: fs-uae-launcher - Graphical configuration frontend and launcher for FS-UAE

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=4028

--- Comment #2 from Andrea Musuruane  ---
https://www.dropbox.com/s/eiohk6tkyogyox1/fs-uae-launcher.spec?dl=0
https://www.dropbox.com/s/8euvzgjj25fm4fk/fs-uae-launcher-2.8.3-1.fc26.src.rpm?dl=0

Changelog:
- Updated to new upstream version

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are the assignee for the bug.___
rpmfusion-developers mailing list -- rpmfusion-developers@lists.rpmfusion.org
To unsubscribe send an email to rpmfusion-developers-le...@lists.rpmfusion.org

[Bug 4028] Review request: fs-uae-launcher - Graphical configuration frontend and launcher for FS-UAE

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=4028

Andrea Musuruane  changed:

   What|Removed |Added

  namespace||free
Summary|fs-uae-launcher - Graphical |Review request:
   |configuration frontend and  |fs-uae-launcher - Graphical
   |launcher for FS-UAE |configuration frontend and
   ||launcher for FS-UAE

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are the assignee for the bug.___
rpmfusion-developers mailing list -- rpmfusion-developers@lists.rpmfusion.org
To unsubscribe send an email to rpmfusion-developers-le...@lists.rpmfusion.org