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

Robert-André Mauchin <zebo...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zebo...@gmail.com



--- Comment #1 from Robert-André Mauchin <zebo...@gmail.com> ---
Hello.

 - Source0 is wrong, it should be:

Source0:       
https://github.com/juanfran/svgo-inkscape/archive/v%{version}/%{name}-%{version}.tar.gz

 - You should ask upstream first to include a license file, not do it yourself.
If upstream is unresponsive, then you might include it.

« Packagers who choose to do this [include text of the license] should ensure
that they have exhausted all attempts to work with upstream to include the
license text as part of the source code, or at least, to confirm the full
license text explicitly with the upstream, as this minimizes the risk on the
packager. Packagers should also take copies of license texts from reliable and
canonical sources (such as the Fedora Software Licenses page, the FSF licenses
page, or the OSI license list), whenever possible. »

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

 - This does not replace shebangs, this change line encoding of the file!

#Replace shebangs line #!/usr/bin/env python
sed -i 's/\r//' %{name}-%{version}/%{name}/svgo.inkscape.py

   Just mark the file as executable and brp-mangle-shebangs will automatically
do the rest:

chmod 0755 %{name}/svgo.inkscape.py


 -  -c %{name} is not necessary in %autosetup

 - Then %install should be simplified to:

%install
install -Dpm 0644 %{name}.inx -t %{buildroot}%{_datadir}/inkscape/extensions/
cp -pr  %{name} -t %{buildroot}%{_datadir}/inkscape/extensions/


    And %doc in %changelog:

%files
%license LICENSE.txt
%doc README.md

 - There's a mix of tabs and spaces used for indentation, use one or another
not both.

 - This package won't work as intended: first if you read the Python source you
see that it is expecting "node" in a subdirectory:

    def effect(self):
        command = "./node/bin/node svgo.js --file=" + self.args[0]

   This should be patched to depend on the system-wide node ( and you should
thus add a Requires for it).

 - Second svgo.js depends on svgo itself, which is a node module. That's why
there is a package.json provided. You should thus run "npm install" in the
svgo-inkscape directory to install the required modules.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

Reply via email to