[Bug 1429226] Review Request: dataagregator - Program for extracting information from multiple remote servers via ssh simultaneously

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

 



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



--- Comment #4 from Milovidov Mikhail <milovidovmikhail@xxxxxxxxx> ---
(In reply to VincentS from comment #3)
> Hello,
> I'm not yet a packager, so this is an unofficial review.
> 
> 
> Package Review
> ==============
> 
> REVIEW:
> 
> + OK
> - NA 
> X ISSUE
> 
> + Package meets naming and packaging guidelines.
> + Spec file matches base package name.
> + Spec has consistant macro usage.
> + Meets Packaging Guidelines.
> + License
> + License field in spec matches
> ^^
> I'm not sure about license files.
> I haven't seen information about MIT, GPLv3+ or BSD.
> 
> + License file included in package
> + Spec in American English
> + Spec is legible.
> + Sources match upstream sha256sum:
> $ sha256sum 0.9.2.tar.gz rpmbuild/SOURCES/dataagregator-0.9.2.tar.gz 
> 2df8316e0b9eab76425f83c38a79f9f6a341767803ca268fe571ed2c8fc1871f 
> 0.9.2.tar.gz
> 2df8316e0b9eab76425f83c38a79f9f6a341767803ca268fe571ed2c8fc1871f 
> rpmbuild/SOURCES/dataagregator-0.9.2.tar.gz
> 
> X Package needs ExcludeArch
> ^^
> I think you may add exclude arch, as you can see in the following link.
> https://koji.fedoraproject.org/koji/taskinfo?taskID=18275674
> 
> + BuildRequires correct
> - Spec handles locales/find_lang
> - Package is relocatable and has a reason to be.
> + Package has %defattr and permissions on files is good.
> $ rpmls 
> /var/lib/mock/fedora-rawhide-x86_64/result/dataagregator-0.9.2-4.fc27.x86_64.
> rpm -rwxr-xr-x  /usr/bin/DataAgregator
> drwxr-xr-x  /usr/share/doc/dataagregator
> -rw-r--r--  /usr/share/doc/dataagregator/README.md
> drwxr-xr-x  /usr/share/licenses/dataagregator
> -rw-r--r--  /usr/share/licenses/dataagregator/COPYING
> -rw-r--r--  /usr/share/licenses/dataagregator/license.txt
> 
> + Package is code or permissible content.
> + Doc subpackage needed/used.
> + Packages %doc files don't affect runtime.
> 
> - Headers/static libs in -devel subpackage.
> - Spec has needed ldconfig in post and postun
> - .pc files in -devel subpackage/requires pkgconfig
> - .so files in -devel subpackage.
> - -devel package Requires: %{name} = %{version}-%{release}
> - .la files are removed.
> 
> - Package is a GUI app and has a .desktop file
> 
> + Package compiles and builds on at least one arch.
> + Package has no duplicate files in %files.
> + Package doesn't own any directories other packages own.
> + Package owns all the directories it creates.
> X Rpmlint output:
> $ rpmlint rpmbuild/SPECS/dataagregator.spec
> /var/lib/mock/fedora-rawhide-x86_64/result/dataagregator-0.9.2-4.fc27.src.
> rpm
> /var/lib/mock/fedora-rawhide-x86_64/result/dataagregator-0.9.2-4.fc27.x86_64.
> rpm 
> rpmbuild/SPECS/dataagregator.spec:16: W: unversioned-explicit-provides
> bundled(qsshlib)
> rpmbuild/SPECS/dataagregator.spec:30: W: setup-not-quiet
> dataagregator.src: E: summary-too-long C Program for extracting information
> from multiple remote servers via ssh simultaneously
> dataagregator.src: W: spelling-error %description -l en_US agregation ->
> aggregation, segregation, congregation
> dataagregator.src: W: spelling-error %description -l en_US json -> son, j son
> dataagregator.src:16: W: unversioned-explicit-provides bundled(qsshlib)
> dataagregator.src:30: W: setup-not-quiet
> dataagregator.x86_64: E: summary-too-long C Program for extracting
> information from multiple remote servers via ssh simultaneously
> dataagregator.x86_64: W: spelling-error %description -l en_US agregation ->
> aggregation, segregation, congregation
> dataagregator.x86_64: W: spelling-error %description -l en_US json -> son, j
> son
> dataagregator.x86_64: W: no-manual-page-for-binary DataAgregator
> 2 packages and 1 specfiles checked; 2 errors, 9 warnings.
> ^^
> Just a few little errors need to be solved.
> 
> + final provides and requires are sane.
> 
> 
> SHOULD Items:
> 
> + Should build in mock.
> + Should build on all supported archs
> - Should function as described.
> - Should have sane scriptlets.
> - Should have subpackages require base package with fully versioned depend.
> + Should have dist tag
> + Should package latest version
> - check for outstanding bugs on package. (For core merge reviews)
> 
> Issues:
> 
> 1. I think your package needs ExcludeArch 
> 2. rpmlint errors
> 
> If you would like to be sponsored, I advise you to do informal review on
> other review requests, you need to show you're interested in packaging and
> you need to know packaging [0] and reviewing [1] guidelines.
> 
> [0] https://fedoraproject.org/wiki/Packaging:Guidelines
> [1] https://fedoraproject.org/wiki/Packaging:ReviewGuidelines
> 
> Personnaly, your software interests me, I will try it soon.
> By the way, did you ever try it with ssh key?

Thank you for attention, I will fix your issues in the nearest time.

Utility is not supported ssh keys yet, but this is not a problem to add this
feature in next realease, if somebody will test it. In my work environments,
where I use this utility, the servers are virtuals that reinstalls very often,
and this means that I need to copy my key to all servers each time after
redeploy. Therefore, I cann't test this feature in production mode.

-- 
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]