[Bug 1458394] Re-Review Request: nuvolaruntime - Tight integration of web apps with your desktop, renaming nuvolaplayer

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

 



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

Vít Ondruch <vondruch@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|nobody@xxxxxxxxxxxxxxxxx    |vondruch@xxxxxxxxxx
              Flags|                            |fedora-review+



--- Comment #4 from Vít Ondruch <vondruch@xxxxxxxxxx> ---
* 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.

* 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 ...

* 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).

* 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.

* description
  - I'd keep just the first paragraph of the description (i.e. "Nuvola Apps
    is a runtime for semi-sandboxed web apps ...").


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/

-- 
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