Bug 1419317 (deepin-terminal)

The usual:
- please link to the raw spec file for fedora-review's sake
- underscores on macros are not necessary

It would be great to add an appdata file [https://fedoraproject.org/wiki/Packaging:AppData]. This will make it easier to discover and install deepin-terminal for users of other desktop environments. I'm assuming it's independently usable.

It doesn't build in rawhide and F26 currently:
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./lib/animation.vala:33.9-33.43: 
warning: the modifier `static' is not applicable to constants
        public static const int FAST = 250; /* Good for animations that convey duplicated information */
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./lib/animation.vala:34.9-34.46: 
warning: the modifier `static' is not applicable to constants
        public static const int INSTANT = 150; /* Good for animations that don't convey any information */
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./lib/animation.vala:35.9-35.45: 
warning: the modifier `static' is not applicable to constants
        public static const int NORMAL = 500;
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./lib/animation.vala:36.9-36.44: warning: the modifier `static' is not applicable to constants
        public static const int SLOW = 1000; /* Good for animations that convey information that is only presented in the animation */
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
gee-0.8.vapi:278.3-278.31: error: overriding method `Gee.HashSet.foreach' is incompatible with base method `Gee.AbstractCollection.foreach': incompatible type of parameter 1.
                public override bool @foreach (Gee.ForallFunc f);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
gee-0.8.vapi:278.3-278.31: error: Gee.HashSet.foreach: no suitable method found to override
                public override bool @foreach (Gee.ForallFunc f);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
gee-0.8.vapi:368.3-368.31: error: overriding method `Gee.PriorityQueue.foreach' is incompatible with base method `Gee.AbstractCollection.foreach': incompatible type of parameter 1.
                public override bool @foreach (Gee.ForallFunc f);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
gee-0.8.vapi:368.3-368.31: error: Gee.PriorityQueue.foreach: no suitable method found to override
                public override bool @foreach (Gee.ForallFunc f);
    /builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/quake_window.vala:43.27-43.54: warning: Gdk.Screen.get_monitor_at_window has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/quake_window.vala:43.56-43.79: warning: Gdk.Screen.get_active_window has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/quake_window.vala:45.13-45.39: warning: Gdk.Screen.get_monitor_geometry has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/quake_window.vala:293.27-293.54: warning: Gdk.Screen.get_monitor_at_window has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/quake_window.vala:293.56-293.79: warning: Gdk.Screen.get_active_window has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/quake_window.vala:295.13-295.39: warning: Gdk.Screen.get_monitor_geometry has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/quake_window.vala:177.34-177.61: warning: Gdk.Screen.get_monitor_at_window has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/quake_window.vala:177.63-177.86: warning: Gdk.Screen.get_active_window has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/quake_window.vala:178.34-178.61: warning: Gdk.Screen.get_monitor_at_window has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/quake_window.vala:181.13-181.39: warning: Gdk.Screen.get_monitor_geometry has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/window.vala:56.27-56.54: warning: Gdk.Screen.get_monitor_at_window has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/window.vala:56.56-56.79: warning: Gdk.Screen.get_active_window has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/window.vala:58.13-58.39: warning: Gdk.Screen.get_monitor_geometry has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/config_window.vala:486.43-486.70: warning: Gdk.Screen.get_monitor_at_window has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/config_window.vala:486.72-486.95: warning: Gdk.Screen.get_active_window has been deprecated since 3.22
/builddir/build/BUILD/deepin-terminal-4f7069e064ae4e5437013295fd5bdbfd8867086c/./widget/config_window.vala:488.29-488.55: warning: Gdk.Screen.get_monitor_geometry has been deprecated since 3.22
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Hi Zbigniew Jędrzejewski-Szmek,

Thank your for you help.
I searched this error in the github. The error seems to be related to vala.
`rm vapi/gee-0.8.vapi` should fix it on fc26.

Some Requires may not be needed when using other desktop environments. This requires further testing in the virtual machine.

```
# right-click menu style
Requires:       deepin-menu
# run command by create_from_commandline
Requires:       deepin-shortcut-viewer
Requires:       xdg-utils
Recommends:     deepin-manual
```

Build test: https://copr.fedorainfracloud.org/coprs/mosquito/deepin/build/580989/

For reviewer convenience, whenever you make another version of package under review, please add the "Spec:" and "SRPM:" lines again, so it's easy to go to the latest sources.

rpmlint:
> deepin-terminal.src: W: invalid-license GPL3
The list of allowed license is at https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Software_License_List. In this case, it should be "GPLv3".

> deepin-terminal.x86_64: E: incorrect-locale-subdir /usr/share/locale/es_419/LC_MESSAGES/deepin-terminal.mo
> deepin-terminal.x86_64: E: invalid-lc-messages-dir /usr/share/locale/es_419/LC_MESSAGES/deepin-terminal.mo
It's probably harmless, but indeed seems strange.

> deepin-terminal.x86_64: E: invalid-desktopfile /usr/share/applications/deepin-terminal.desktop file contains group "NewWindow Shortcut Group", but groups extending the format should start with "X-"
> deepin-terminal.x86_64: E: invalid-desktopfile /usr/share/applications/deepin-terminal.desktop file contains group "Quake Shortcut Group", but groups extending the format should start with "X-"

Is this a Deepin thing? If so, you can ignore this warning.

> 3 packages and 0 specfiles checked; 5 errors, 8 warnings.
There's a bunch of other warnings, but I think they are all harmless.

It looks almost OK. We can revisit the appdata question later.

The package has a ~600kB binary, and ~5MB of data. Please split out the data into a noarch subpackage. Everything under /usr/share can go that that subpackage. And then the main package should have Requires = %{name}-data = %{version}-%{release}.

Errr, Requires: %{name}-data = %{version}-%{release} of course.

SPEC:  https://raw.githubusercontent.com/FZUG/repo/master/rpms/deepin_project/deepin-terminal.spec
SRPM:  https://copr-be.cloud.fedoraproject.org/results/mosquito/deepin/fedora-25-x86_64/00582191-deepin-terminal/deepin-terminal-2.5.1-2.git82c4a12.fc25.src.rpm
Task: https://copr.fedorainfracloud.org/coprs/mosquito/deepin/build/582191/

When do i need to split the package? /usr/share/ is very large?

There's no hard rule. For documentation, it's suggested to split it out if it's more than a megabyte or so. For shared data, if it's a few megabytes... It mostly saves some space on mirrors. Another minor advantage is that most likely the data doesn't change much, so drpm will save most of the download.

%description should end in ".".

You don't need %doc README.md and %license LICENSE in the main subpackage, because it's already included in %{name}-data anyway, and %{name}-data is required by the main subpackage.

+ package name is OK
+ license is acceptable for Fedora (GPLv3)
+ license is specified correctly
+ builds and installs OK
+ Provides/Requires/BuildRequires appear to be correct
+ scriptlets are OK (at least I think so, there's many different upgrade/downgrade/install/uninstall paths).

OK, I think it's all good, apart from some cosmetic issues. Package is APPROVED.

Thank you. @Zbigniew Jędrzejewski-Szmek
you are the best.

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/deepin-terminal

This is already in Rawhide. Closing on behalf of the Deepin Desktop packaging effort.