[Bug 1441828] Review Request: nuvolaplayer- Cloud Music Integration for your Linux Desktop

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1441828



--- Comment #35 from mgansser@xxxxxxxx <mgansser@xxxxxxxxx> ---
(In reply to Vít Ondruch from comment #34)
> (In reply to mgansser@xxxxxxxx from comment #32)
> > (In reply to Vít Ondruch from comment #30)
> > > (In reply to Vít Ondruch from comment #21)
> > > > > * What is the purpose of %{_datadir}/nuvolaplayer3/web_apps/test
> > > > >   - Is the content of the directory used somewhere?
> > > > > 
> > > > 
> > > > This is a test service to test various features of Nuvola (nuvola -D -a
> > > > test). It may be helpful when you need provide your users with support. It
> > > > might go to devel subpackage though.
> > > 
> > > Actually, now I understand what it is :) It should be packaged as other
> > > nuvola-app- plugins. IOW this should go into nuvola-app-test subpackage.
> > > 
> > > But for the moment, the -devel package should be good as well. You can move
> > > it into subpackage any time later. IOW some TODO: in .spec file should be
> > > enough ATM.
> > > 
> > 
> > done
> 
> Do I understand it correctly that you leave it in -devel package ATM?
> 

yes i have leave it in the -devel package.

> > > > > * What is the purpose of %{_datadir}/nuvolaplayer3/audio
> > > > >   - Is the .mp3 fie used somewhere for something?
> > > > > 
> > > > 
> > > > The mp3 file is used to check that gstreamer can play mp3 files.
> > > 
> > > Ok, it seems this file is referenced in the code, so it should be preserved
> > > in the package.
> > > 
> > > Actually, testing this now (using the test app included in -devel subpackage
> > > for now), Nuvola works without this file, but apparently, when the file is
> > > available, I am prompted to install mp3 codec.
> > > 
> > > And the behavior is quite funny. If I remove the file, Nuvola just starts.
> > > If I keep that file, asks me to install mp3 coded. When I install the codec,
> > > it keeps asking me for Flash :) Well, this could be improved ...
> > > 
> > > Actually, may be we should add some "gstreamer1-plugin-mpg123", not sure.
> > > Lets polish this later ...
> > > 
> > > 
> > 
> > add recommend gstreamer1-plugin-mpg123
> 
> That is good ... It just worries me a bit, that next in line should be
> recommended flash plugin. But flash is not available in Fedora, so it is not
> good idea to recommend it ... just thinking loud :)
> 

no further commends added.
> > > > * License
> > > >   - Could you please check the licensing? Nuvolaplayer is BSD, but the
> > > >     engine.io is MIT.
> > > 
> > > The license should be just "BSD or MIT" IMO. I can't see any GPL code in
> > > Nuvola.
> > > 
> > 
> > I changed the license field to BSD, or do you mean "BSD or MIT" i never saw
> > this combination.
> 
> Actually I meant "BSD and MIT" ("or" would apply for double licensing, which
> is not the case). The Nuvola itself is BSD licensed, but the bundled
> engineio-soup content is MIT. It is typically good idea to attach some
> clarifying comment above the license tag.
> 
> 

changed it to "BSD and MIT" and add comment above the license tag.

> (In reply to mgansser@xxxxxxxx from comment #33)
> > (In reply to Vít Ondruch from comment #30)
> > 
> > > > > * Bundled engine.io library?
> > > > >   - It seems engine.io library is bundled by nuvolaplayer. Is it required for
> > > > >     something? If yes, shouldn't it be extracted or at least there should be
> > > > >     the "bundled()" provider [4].
> > > > > 
> > > > 
> > > > Engine.io is not a bundled library in the meaning as a copy of an
> > > > independent library. It just hasn't been separated into and independent
> > > > library yet. Engineio is used in some experimental features which are not
> > > > built by default yet but that may change in the future.
> > > 
> > > Frankly, I am not very satisfied with upstream response, since this [2]
> > > commit clearly links engine.io.js file from different upstream repository.
> > > Moreover, the .js file is in two copies currently in the source code ...
> > > 
> > 
> > answer from upstream:
> > I see. I thought you talked about libengineio-soup - the Vala port of
> > engineio. It didn't occur to me there is also engine.io.js. That could be
> > unbundled. However, I have no idea what is the typical filesystem location
> > for JavaScript libraries. Let's continue at #341.
> > Ticket: https://github.com/tiliado/nuvolaplayer/issues/341
> 
> These [1] are JavaScript guidelines which applies for Fedora if you decide
> to unbundle. If you decide to keep it bundle, then you should follow these
> guidelines [2].
> 
> 
> 
> [1]: https://fedoraproject.org/wiki/Packaging:JavaScript
> [2]:
> https://fedoraproject.org/wiki/Bundled_Libraries#Requirement_if_you_bundle

i keep the library bundled for now

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/nuvolaplayer.spec
SRPM URL:
https://martinkg.fedorapeople.org/Review/SRPMS/nuvolaplayer-3.1.3-5.fc25.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux