[Bug 1424890] Review Request: domterm - terminal emulator based on web technologies

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

 



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



--- Comment #15 from Andrew Toskin <andrew@xxxxxxx> ---
Your spec file still has several Source0 tags, all but one commented out.
Again, the macro-ized URL we would normally expect for a project hosted on
GitHub is this:

  https://github.com/PerBothner/DomTerm/archive/%{version}.tar.gz

One issue with this, though, is that the project's GitHub seems to be a bit
behind the source you're using to build the SRPM. You're building DomTerm 0.74,
but the latest release I see tagged on GitHub is still version 0.72.

If you do use that URL, you'd probably also have to update the directory name
used in the autosetup line (currently, `%autosetup -n DomTerm-%{commit0}`). Or
if, as the %{commit} macro suggests, you plan to build a prerelease from
arbitrary commits, then you'd use this very similar macro-ized URL:

  https://github.com/PerBothner/DomTerm/archive/%{commit0}.tar.gz

And then you probably ought to change the spec Release tag to reflect the fact
that it's a prerelease (that is, use a number like 0.1 instead of 1).


The most recent entry in the spec %changelog is a little behind, as it mentions
version 0.72-1, rather than 0.74-1. I don't think it's super important to have
a detailed spec changelog before this package has been initially approved, but
rpmlint complains when the spec Version tag and the version mentioned in the
latest changelog entry are out of sync. At least *replace* the changelog entry,
if not adding new ones --- until the package is approved. *Then* you should
keep the several most recent entries, so users can at their option check the
changelog before they install updates.


In the %build section, I noticed this line:

  %configure --disable-pty --with-qtwebengine --with-java --with-libwebsockets

If you're configuring to build with java, shouldn't java be a Requires instead
of a Recommends?


I think .desktop files should be installed using desktop-file-install and/or
desktop-file-validate, to ensure they validate.

rpmlint caught an issue anyway, though: the .desktop file contains the line
`Exec=domterm`, but there is no `domterm` file under /usr/bin/ -- just ldomterm
and qtdomterm.


The built domterm package contains the directory /usr/lib/.build-id/  -- which
looks to me like leftovers from the build. Probably this isn't needed in the
binary RPM, right?


fedora-review detects some directories without a known owner. Add macro-ized
versions of these to your %files section.

  /usr/share/domterm/defaults/
  /usr/share/domterm/defaults/preferences/


The spec License tag simply says "BSD". Since there are so many variants on the
BSD license, I don't think that's specific enough. You would need to use
"BSD-simplified" or whatever the standard abbreviation for the current license
is...

Moreover, fedora-review's license check finds 3 files in /qtdomterm/ which have
comments stating that they use the GNU GPL, which I think might mean that all
of DomTerm should also use GPL(?).

I also suspect that all these files containing copied code might amount to
library bundling, which is at least discouraged in Fedora. But I'll let
Zbigniew have the final say on this.

-- 
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 Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]