https://bugzilla.redhat.com/show_bug.cgi?id=1458394 --- Comment #5 from mgansser@xxxxxxxx <mgansser@xxxxxxxxx> --- (In reply to Vít Ondruch from comment #4) > * Please describe patches > - You carry aroun nuvoplaruntie-wscript patch, but what is it purpose? > Where > is it coming from? Is it upstreamed or taken from upstream? While you > probably know, it would be helpful to others to attache 2 line comment > above the patch to explain the purpose and link the sources. Thx. add description > * appindicator dependency > - Is it mandatory? Is it possible to disable it? I don't think this is > supported by default in Fedora. As far as I understand it, you need Gnome > Shell KStatusNotifierItem/AppIndicator Support plugin [1] installed to > benefit from this ... add configure option to disable appindicator > * Space characters on the end of line > - There are several lines containing empty space after the end of line. > This > is just minor nit, but probably better to avoid this (the spaces are > nicely > visualized by colorized git diff output). removed spaces at EOL > * waf vs waf-3 > - While the "waf-3" have to be required to pull in the Python3 version of > waf, > I think it should be fine to use the "waf" command. Or do you prefer to > be > explicit about it? I leave it up to you. will take waf when commit it to git > * description > - I'd keep just the first paragraph of the description (i.e. "Nuvola Apps > is a runtime for semi-sandboxed web apps ..."). > shorten the description as you mentioned > Otherwise the package is quite similar to the nuvolaplayer (not > surprisingly) and I don't see anything major => APPROVED > > > [1]: https://extensions.gnome.org/extension/615/appindicator-support/ thanks for your review Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/nuvolaruntime.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/nuvolaruntime-4.4.0-3.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
