[Bug 871191] Review Request: sendKindle - CLI tool for sending files via email to your Amazon Kindle device

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

 



Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=871191

Kamil Páral <kparal@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW

--- Comment #4 from Kamil Páral <kparal@xxxxxxxxxx> ---
(In reply to comment #3)
> So first a few quick things:
> 1. Your source0 url should be:
> https://github.com/kparal/%{name}/tarball/v%{version}#/%{name}-%{version}.
> tar.gz

It seems that Github was doing some changes lately. The URL that you posted
works, but I no longer see it accessible from the web interface. It also has
some drawbacks - the file name contains a git commit and other information
(like "kparal-sendKindle-v2-0-g3bd4abf.tar.gz"), and the top level directory as
well ("kparal-sendKindle-3bd4abf/"). I had problems work with that using
Source0 and %setup, I would probably have to repackage it completely. Therefore
I assumed manual git export is just easier (and Mirek Suchý advised me the
same).

But today I see Github offering me this link:
https://github.com/kparal/sendKindle/archive/v2.tar.gz

That is nearly perfect. The filename is static now ("v2.tar.gz"), no git commit
hash, unfortunately it doesn't contain a project name so it will mess up my
SOURCES/. (I don't know how to deal with that, any advice welcome). But the top
level directory is now finally reasonable ("sendKindle-2/"). So overall I think
it's a good enough change and I switched Source0 to this URL (the old method is
just commented out, I'll remove it later).

fedora-review now says:
[!]: SourceX / PatchY prefixed with %{name}.
     Note: Source0 (v2.1.tar.gz)

Isn't is possible to somehow say that the downloaded file should be renamed?

> 
> 2. Instead of:
> # installation removed the executable bit for some reason, but rpmlint then 
> # complains (the file has a hashbang), so put the X bit back again
> chmod a+x %{buildroot}%{python_sitelib}/sendKindle.py
> 
> do:
> sed -i '1d' sendKindle.py (in prep section)

Done (a bit more secure variant).

> 
> You are missing requires on python. 

I thought that rpmbuild does that automatically. Isn't this enough?

$ rpm -qRp /var/lib/mock/fedora-17-x86_64/result/sendKindle-2-1.fc17.noarch.rpm
/usr/bin/python  
python(abi) = 2.7
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PartialHardlinkSets) <= 4.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsXz) <= 5.2-1


> Also as upstream you might want to
> considering supporting python 3.

I actually already received a patch from someone. But I don't want to maintain
two separate versions. Once Fedora switches to Python 3, I'll definitely adjust
the program.

> 
> as for licensing, package is GPLv3+ and part 4 of the license states that
> "...;and give all recipients a copy of this License along with the
> Program.". You should therefore both as upstream and packager distribute
> text of the license together with the source code.

Actually I think this (and all other paragraphs) do not apply for the software
author. The packaging guidelines also seem to indicate that (A)GPL does not
require adding the very text of the license:
"Common licenses that require including their texts with all derivative works
include ASL 2.0, EPL, BSD and MIT."
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines

But that's just nitpicking :-) I included the license text in the upstream code
and in the RPM.


I have updated the files:
Spec URL: http://kparal.fedorapeople.org/pkgs/sendKindle/sendKindle.spec
SRPM URL:
http://kparal.fedorapeople.org/pkgs/sendKindle/sendKindle-2.1-1.fc17.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]