https://bugzilla.redhat.com/show_bug.cgi?id=1546459 --- Comment #2 from Luya Tshimbalanga <luya@xxxxxxxxxxxxxxxxx> --- (In reply to Robert-André Mauchin from comment #1) > Hello. > > - Source0 is wrong, it should be: > > Source0: > https://github.com/juanfran/svgo-inkscape/archive/v%{version}/%{name}- > %{version}.tar.gz Fixed > - You should ask upstream first to include a license file, not do it > yourself. If upstream is unresponsive, then you might include it. Already filed the request: https://github.com/juanfran/svgo-inkscape/issues/6 > - 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 Fixed > > - -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. Fixed. > > - 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). BuildRequires: nodejs-packaging seems the suggestion according to the guideline. > > - 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. Looking at the nodejs guideline https://fedoraproject.org/wiki/Packaging:Node.js?rd=Node.js/Packagers#Installing_Modules, it looks like using "npm install" is discouraged. Is there a better method instead? -- 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
