https://bugzilla.redhat.com/show_bug.cgi?id=1458394
--- Comment #5 from mgansser(a)alice.de <mgansser(a)online.de> ---
(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...
--
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