Spec URL: http://hiemanshu.fedorapeople.org/SPECS/security-menu.spec SRPM URL: http://hiemanshu.fedorapeople.org/SRPMS/security-menu-1.0-1.fc12.src.rpm Description: This Package adds a Security Lab sub-menu to the xdg menu structure for GNOME and LXDE.
This package still needs a lot of work, nevertheless I'm going to review it because I suggested something similar half a year ago.
I am fixing a couple of issues I missed, I will update the links when the new release is uploaded.
Spec URL is the same, SRPM URL : http://hiemanshu.fedorapeople.org/SRPMS/security-menu-1.0-2.fc12.src.rpm Link to koji scratch build : http://koji.fedoraproject.org/koji/taskinfo?taskID=1880070
Updated, Fixed conflict with other packages SRPM URL : http://hiemanshu.fedorapeople.org/SRPMS/security-menu-1.0-3.fc12.src.rpm Link to Koji scratch build : http://koji.fedoraproject.org/koji/taskinfo?taskID=1880103
Final Release, Fixed all issues with package, SPEC URL is the same, SRPM : http://hiemanshu.fedorapeople.org/SRPMS/security-menu-1.0-4.fc12.src.rpm Link to koji scratch build : http://koji.fedoraproject.org/koji/taskinfo?taskID=1880993
Hi, I made a couple of changes to the source and checked them an at https://fedorahosted.org/security-spin/browser/security-menu Changes: * Rename X-Security-Lab to X-SecurityLab for better fdo compliance. Whitespces are usually just left out. * Add subcategories X-Anonymity, X-CodeAnalysis, X-Forensics, X-IntrusionDetection, X-Password-Tools, X-Reconnaissance and X-Wireless * Use TryExec to determine installed apps, see http://standards.freedesktop.org/desktop-entry-spec/latest/ar01s05.html * Menu tweaks * Enhance Makefile * Added AUTHORS * Added ChangeLog (for upstream changes, not packaging). Please review if my info is right! Please take a look the changes, especially what I have done for the applications included with <filename> </filename>. I think this might be needed for more apps, please check. When we are done with everything, we tag it 1.0 in git make a tarball to package. The spec looks sane, but * you should use desktop-file-validate in %install, see https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files * Don't forget to include AUTHORS and ChangeLog * Change the Source0 url to point to fedorahosted when we have a tarball there Please don't upload the rpms into git, just put them on your fedorapeople.org account while they are under review.
Some more comments on the spec: drop %define ..., it is not needed. If you need such things, use %global instead, see https://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define Drop Requires(pre) and add Requires: redhat-menus for directory ownership. As long as we have no icons, remove the icon-cache scriptlets. When we have, add the latest version of the scripts again. Yours are slightly outdated compared to https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache
Can you please update the source package and make a release of it? Look at the changes I did in git, there are more packages that need to be fixed. After that, please update the spec with the changes from the comments above, so I can review the package. If you have any questions, don't hesitate to ask.
I have been out on vacation, I will take care of these as soon as possible. Regards, Hiemanshu Sharma.
I have updated with regards to your comments, Thank you for all the help. Here are the updated files : SPEC URL: http://hiemanshu.fedorapeople.org/SPECS/security-menu.spec SRPM URL: http://hiemanshu.fedorapeople.org/SRPMS/security-menu-1.0-5.fc12.src.rpm Link to koji scratch build : http://koji.fedoraproject.org/koji/taskinfo?taskID=1916324 Source0 is now pointed to : https://fedorahosted.org/released/security-spin/security-menu-1.0.tar.gz If you need any more changes please let me know. Hiemanshu Sharma.
(In reply to comment #10) > If you need any more changes please let me know. tested the rpm and i missed several submenus - rebuild with patched security-lab.menu - https://fedorahosted.org/security-spin/browser/security-lab.menu.patch is working to make the missing submenus appear
I have updated with regards to your comments. Here are the updated files : SPEC URL: http://hiemanshu.fedorapeople.org/SPECS/security-menu.spec SRPM URL: http://hiemanshu.fedorapeople.org/SRPMS/security-menu-1.0-6.fc12.src.rpm Link to koji scratch build : http://koji.fedoraproject.org/koji/taskinfo?taskID=1919661 If you need any more changes please let me know. Hiemanshu Sharma.
Review for 1c3e5f77895ce44ba7a028d7a0f86d72 security-menu-1.0-6.fc12.src.rpm OK - MUST: $ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/security-menu-1.0-6.fc13.* 2 packages and 0 specfiles checked; 0 errors, 0 warnings. OK - MUST: named according to the Package Naming Guidelines OK - MUST: spec file name matches the base package %{name} OK - MUST: package meets the Packaging Guidelines OK - MUST: Fedora approved license and meets the Licensing Guidelines: GPLv2 OK - MUST: License field in spec file matches the actual license: GPLv2 OK - MUST: license file included in %doc OK - MUST: spec is in American English OK - MUST: spec is legible FIX - MUST: source doesn't match the upstream source by MD5 from SRPM: 89de4bf505dab16f51a8779fc273a7d1 from Source0 URL: 8d8113dc8009a6ddc002ba56ae18891b OK - MUST: successfully compiles and builds into binary rpms on x86_64 OK - MUST: No ExcludeArch. OK - MUST: all build dependencies are listed in BuildRequires. N/A - MUST: handles locales properly with %find_lang N/A - MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. N/A - MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review OK - MUST: owns all directories that it creates (none) OK - MUST: no duplicate files in the %files listing OK - MUST: Permissions on files are set properly, includes %defattr(...) OK - MUST: package has a %clean section, which contains rm -rf %{buildroot} ( or $RPM_BUILD_ROOT ). OK - MUST: consistently uses macros OK - MUST: package contains code, or permissable content N/A - MUST: Large documentation files should go in a -doc subpackage OK - MUST: Files included as %doc do not affect the runtime of the application N/A - MUST: Header files must be in a -devel package N/A - MUST: Static libraries must be in a -static package N/A - MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A - MUST: If a package contains library files with a suffix, then library files that end in .so must go in a -devel package. N/A - MUST: devel packages must require the base package using a fully versioned dependency OK - MUST: The package does not contain any .la libtool archives. OK - MUST: All desktop files are validated with desktop-file-install in the %install section. OK - MUST: package does not own files or directories already owned by other packages. OK - MUST: at the beginning of %install, the package runs rm -rf %{buildroot} OK - MUST: all filenames valid UTF-8 SHOULD Items: OK - SHOULD: Source package includes license text(s) as a separate file. N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available. OK - SHOULD: builds in mock. OK - SHOULD: compiles and builds into binary rpms on all supported architectures. OK - SHOULD: functions as described. N/A - SHOULD: Scriptlets are used, those scriptlets must be sane. N/A - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. N/A - SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg N/A - SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself. Other items: OK - latest stable version OK - SourceURL valid Please download the source again to make sure the md5 matches.
One more thing I forgot: We should rename the whole thing to "security-menus" (Plural), because all other packages are plural too.
I fixed the problem with the md5sum and changed the name as well. SPEC URL : http://hiemanshu.fedorapeople.org/SPECS/security-menus.spec SRPM URL : http://hiemanshu.fedorapeople.org/SRPMS/security-menus-1.0-7.fc12.src.rpm Koji Scratch Build : http://koji.fedoraproject.org/koji/taskinfo?taskID=1923035 Please let me know if anything else needs to be done. Regards, Hiemanshu Sharma.
renaming ok, md5sums ok, you are sponsored, everything is fine. One final note: When downloading a file please use a download manager that preservers timestamps, because usually the timestamps in the srpm should be the same as the Source0 URL, see https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps I guess you first made the tarball and then uploaded it to fedorahosted, so it's not really important in this case. The package is APPROVED. Good job!
Hiemanshu, you should request CVS, see http://fedoraproject.org/wiki/CVS_admin_requests Doing that for you now, so we cam move on New Package CVS Request ======================= Package Name: security-menus Short Description: Menu Structure for the Security Spin Owners: hiemanshu cwickert Branches: F-11 F-12 InitialCC:
CVS done (by process-cvs-requests.py).
Hiemanshu, can this be closed now?
Package Change Request ====================== Package Name: security-menus New Branches: epel7 el6 Owners: fab InitialCC:
Git done (by process-git-requests).