Spec URL: http://fpaste.org/tnmQ/ SRPM URL: http://rghost.net/9723221 Description: A Wicd client built on the KDE Development Platform.
Spec file checked http://fpaste.org/cbsN/ SRPM URL: http://rghost.net/9732771
I'm sorry. This is the last second fix :) Spec URL: http://fpaste.org/npzz/ SRPM URL: http://rghost.net/9737101 http://koji.fedoraproject.org/koji/taskinfo?taskID=3115413
I'm using KDE and wicd. But I'm not sponsor. Few comments: Use desktop-file-install for desktop files: http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files Please insert license text to package. you build package for KDE4 so you need read https://fedoraproject.org/wiki/SIGs/KDE/Packaging/Guidelines. There are some KDE-specific macros. Please use these instead of the standard macros. P.S. %{_prefix}/libexec == %{_libexecdir}
>Use desktop-file-install for desktop files: >http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files Fixed >Please insert license text to package. A license text is already in the file COPYING >you build package for KDE4 so you need read >https://fedoraproject.org/wiki/SIGs/KDE/Packaging/Guidelines. There are some >KDE-specific macros. Please use these instead of the standard macros. >P.S. %{_prefix}/libexec == %{_libexecdir} Fixed Spec URL: http://fpaste.org/VHLr/ SRPM URL: http://koji.fedoraproject.org/koji/getfile?taskID=3118421&name=wicd-kde-0.2.2-1.fc15.src.rpm
Also this is my first package, and I'm seeking a sponsor.
Spec file and SRPM package are moved to the permanent repository https://github.com/Ignotus/wicd-kde-fedora
Added kde-review flag. For convenience plese provide direct links for spec and srpm they could be wget-ed. Desktop-file-install validate .desktop too. There is no need to call desktop-file-validate after it. Are you sure wicd-kde.desktop couild be used both as application .desktop and as service .desktop? PS I'm not sponsor either.
>Added kde-review flag. thanks >For convenience plese provide direct links for spec and srpm they could be >wget-ed. Spec URL: https://raw.github.com/Ignotus/wicd-kde-fedora/master/wicd-kde.spec SRPM URL: https://github.com/Ignotus/wicd-kde-fedora/blob/master/wicd-kde-0.2.2-1.fc15.src.rpm?raw=true >Desktop-file-install validate .desktop too. There is no need to call >desktop-file-validate after it. fixed >Are you sure wicd-kde.desktop couild be used both as application .desktop and >as service .desktop? In CMakeList.txt: install( PROGRAMS wicd-kde.desktop DESTINATION ${XDG_APPS_INSTALL_DIR} ) install( FILES wicd-kde.desktop DESTINATION ${AUTOSTART_INSTALL_DIR} ) What about kcm_wicd.desktop, it's my mistake. I have fixed it.
%make_install defines DESTDIR var itself. Check it. Speration two desktop-file-install with empty line will add legibility. You had not mentioned any changes done with spec file in the %changelog section (with Release number update). And, I think, that is all I can do for this package.
>%make_install defines DESTDIR var itself. Check it. >Speration two desktop-file-install with empty line will add legibility. fixed >You had not mentioned any changes done with spec file in the %changelog section >(with Release number update). this package's not yet in the fedora repository. So it's not necessary to mention any changes in the %changelog section.
> this package's not yet in the fedora repository. So it's not necessary to > mention any changes in the %changelog section. Actually, you should start maintaining the changelog as soon as you submit the first version for review. That allows reviewers to track what review comments you already addressed and which are still open.
>Actually, you should start maintaining the changelog as soon as you submit the >first version for review. That allows reviewers to track what review comments >you already addressed and which are still open. Spec URL: https://raw.github.com/Ignotus/wicd-kde-fedora/aa975b605f713d72d97d5b108f17989a044cf322/wicd-kde.spec SRPM URL: https://github.com/Ignotus/wicd-kde-fedora/blob/aa975b605f713d72d97d5b108f17989a044cf322/wicd-kde-0.2.2-1.fc15.src.rpm?raw=true
And again, Release number update: 0.2.2-1 -> 0.2.2-2.
>And again, Release number update: 0.2.2-1 -> 0.2.2-2. Spec URL: https://github.com/Ignotus/wicd-kde-fedora/blob/fa03d59c4413288f8eb519289e0b5e984d18f5db/wicd-kde.spec SPRM URL: https://github.com/Ignotus/wicd-kde-fedora/blob/fa03d59c4413288f8eb519289e0b5e984d18f5db/wicd-kde-0.2.2-2.fc15.src.rpm
Could you provide direct download links, please? -rpmlint wicd-kde-0.2.2-2.fc17.i686.rpm ================================================================================ wicd-kde.i686: W: non-conffile-in-etc /etc/dbus-1/system.d/org.kde.wicdclient.scripts.conf wicd-kde.i686: W: no-manual-page-for-binary wicd-kde 1 packages and 0 specfiles checked; 0 errors, 2 warnings. -org.kde.wicdclient.scripts.conf must be marked as a config file. -Why you don't use the kcm_wicd.desktop in the source file? By the way, there is a new wicd-kde release, consider upgrading your package -the package contains a so file, you must call ldconfig
-org.kde.wicdclient.scripts.conf must be marked as a config file. -the package contains a so file, you must call ldconfig fixed SPEC: https://raw.github.com/Ignotus/wicd-kde-fedora/f37c38f037d9f1bcab805467d03dd754f7e52be1/wicd-kde.spec SRPM: https://github.com/Ignotus/wicd-kde-fedora/blob/f37c38f037d9f1bcab805467d03dd754f7e52be1/wicd-kde-0.2.3-1.fc15.x86_64.rpm?raw=true -Why you don't use the kcm_wicd.desktop in the source file? By the way, there is a new wicd-kde release, consider upgrading your package because: >> error: key "Exec" is present in group "Desktop Entry", but the type is "Service" while this key is only valid for type "Application"
> -the package contains a so file, you must call ldconfig Wrong. The package does not install any .so file to %{_libdir}, so ldconfig is NOT needed and should NOT be called. ldconfig does not operate on %{_kde4_libdir}/kde4 = %{_libdir}/kde4, which is only for dlopened plugins. Please remove the ldconfig calls again. > -Why you don't use the kcm_wicd.desktop in the source file? By the way, there > is a new wicd-kde release, consider upgrading your package > > because: > >> error: key "Exec" is present in group "Desktop Entry", but the type is > >> "Service" while this key is only valid for type "Application" There is no need to run desktop-file-validate on .desktop files in %{_kde4_datadir}/kde4/services. Those files will only ever be read by kdelibs, so if kdelibs understands them, they're fine. AIUI, it's normal for KCMs to have an Exec= line which runs kcmshell4 on them, I don't think removing it is a good idea.
Hey Minh Ngo, are you still around? Kevin found some issues.
>Hey Minh Ngo, are you still around? Kevin found some issues. Yes. I'm here :) fixed https://raw.github.com/Ignotus/wicd-kde-fedora/d95dc76dc9f3dd10e6761be9d2f5cf4c9ac1b875/wicd-kde.spec
great, just a quick look: 1. follow the proper review request layout (like in comment 17, add SPEC: and SRPM:) 2. make a changelog entry in the spec every time you change something
SPEC: https://raw.github.com/Ignotus/wicd-kde-fedora/cc0799b8e90a4d27a77b5e3979e376d859687c56/wicd-kde.spec SRPM: https://github.com/Ignotus/wicd-kde-fedora/blob/cc0799b8e90a4d27a77b5e3979e376d859687c56/wicd-kde-0.2.3-2.fc15.src.rpm?raw=true
rpmlint wicd-kde-0.2.3-2.fc17.i686.rpm ================================================================================ wicd-kde.i686: W: conffile-without-noreplace-flag /etc/dbus-1/system.d/org.kde.wicdclient.scripts.conf wicd-kde.i686: W: no-manual-page-for-binary wicd-kde 1 packages and 0 specfiles checked; 0 errors, 2 warnings. -use %config(noreplace) (see packaging guideline).But to be honest: I'm not exactly sure here, maybe the file should be replaced when a upgrade takes place. wicd-kde.src:73: W: macro-in-%changelog %make_install -better don't put a %make_install in the changelog -also the Buildroot tag seems to be wrong - if you really want to make it epel compatible: http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#Distribution_specific_guidelines
If you want to refer to macros in the changelog, you have to double the % sign (which escapes the % sign and outputs one % sign to the user-visible changelog).
SRPM: https://github.com/Ignotus/wicd-kde-fedora/blob/b810d00b9a1cf30b96cfda6b592f105c31dee9c4/wicd-kde-0.2.3-2.fc15.src.rpm?raw=true SPEC: https://raw.github.com/Ignotus/wicd-kde-fedora/b810d00b9a1cf30b96cfda6b592f105c31dee9c4/wicd-kde.spec
The build root tag is still wrong but for now you should try to find a sponsor. Take part in other reviews, demonstrate your knowledge, as you know already, probably.
I can be the sponsor. Gregor, do you think the package is fine to be approved now (except for BuildRoot, which is ignored by current RPM anyway, so I don't think this should be a blocker)? If so, I'll have a quick look to make sure everything is right and then approve it.
@Kevin Yes, go ahead. Only one question: It appears that wicd doesn't conflict with networkmanager. Can we do something about it? As far as I know you can not use them both together.
For that, we need to file a bug against the core packages (IMHO against wicd which is the non-default one), can you file one?
uh oh...this topic has already been discussed: https://bugzilla.redhat.com/show_bug.cgi?id=713109 "I wanted to add a Conflict line, but the package reviewers disagreed. Technically, the packages can be installed at the same time, so there's not really a conflict. You can only run one or the other." Should I fill out a report nevertheless?
@Gregor @Kevin Thank you for helping
Re the Conflicts, I don't care either way. The files don't conflict but the functionality does. But in any case, that is an issue with the wicd core and not related to this review. Unfortunately, I see some issues remaining with the specfile: > BuildRoot: %{_tmppath}/%{name}-%{version} As Gregor already pointed out, please either remove this line or use one of the lines from: https://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag (The RPM in current versions of Fedora ignores BuildRoot entirely.) > BuildRequires: qt-devel Better use qt4-devel, which: * also works on old versions where Qt 3 was the default, * will also work on future versions where Qt 5 will be the default and * allows specifying a version without an Epoch, i.e. these will work if some future version of wicd-kde starts requiring Qt 4.8 for whatever reason: BuildRequires: qt4-devel >= 4.8.0 or: BuildRequires: qt-devel >= 1:4.8.0 but this will not: BuildRequires: qt-devel >= 4.8.0 and the Epoch tends to get forgotten in such cases. > BuildRequires: kdebase-devel Are you sure you want kdebase-devel? I think you want kdelibs-devel, or better kdelibs4-devel (same as for Qt above). > %make_install -C %{_target_platform} The %make_install macro should not be used in Fedora (except for broken legacy tarballs which do not support DESTDIR nor INSTALL_ROOT, but all CMake-based projects support DESTDIR), please use: make install/fast DESTDIR="%{buildroot}" -C %{_target_platform} instead. (For non-CMake projects, use just "install" instead of "install/fast". "install/fast" is a CMake target which skips the checks for whether everything to be installed is already built. We already run "make" in %build, so we can rely on everything already being built by the time we get to %install.) And you should use either %{buildroot} or ${RPM_BUILD_ROOT} consistently rather than mixing the two.
Or I guess: make install/fast DESTDIR=%{buildroot} -C %{_target_platform} without the quotes is good enough (and consistent with the other uses of %{buildroot}).
SPEC: https://raw.github.com/Ignotus/wicd-kde-fedora/fafc7ad3e4a5e684903ddc7c83315d0b9eaa1a60/wicd-kde.spec SRPM: https://github.com/Ignotus/wicd-kde-fedora/blob/fafc7ad3e4a5e684903ddc7c83315d0b9eaa1a60/wicd-kde-0.2.3-3.fc16.src.rpm?raw=true
Pedantic comment of the day: > cp ${RPM_BUILD_ROOT}%{_kde4_datadir}/applications/kde4/%{name}.desktop \ > ${RPM_BUILD_ROOT}%{_kde4_datadir}/autostart/%{name}.desktop We prefer cp -p (no clobbering of the timestamp) to plain cp. Though in this case it doesn't matter because the file has already been edited by the desktop-file-install --add-category command before. So this is only a SHOULD. So it looks like all the issues found have been addressed. Thanks to everyone who did preliminary reviews. The package is APPROVED. We can now proceed with the sponsoring process. Have you already signed up for a FAS (Fedora Account System) account? What's your FAS account name?
>What's your FAS account name? ignotusp
Congratulations, you are now sponsored! Please proceed with the remainder of the: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers process. For any future package submissions, follow the: https://fedoraproject.org/wiki/New_package_process_for_existing_contributors process instead, as you are now an existing package maintainer.
How to create the git repository for wicd-kde?
By following yet another process… https://fedoraproject.org/wiki/Package_SCM_admin_requests but don't worry, you only have to post a message to this bug complying to the format (careful there, those requests are processed by scripts, so there's little to no tolerance for incorrectly-formatted requests) and set the fedora-cvs flag.
New Package SCM Request ======================= Package Name: wicd-kde Short Description: A Wicd client built on the KDE Development Platform Owners: ignotusp Branches: f14 f15 f16 el6 InitialCC:
Git done (by process-git-requests). Not currently accepting new packages for F-14. Kevin, please take ownership of review BZs. Thanks!
> Not currently accepting new packages for F-14. FYI, Fedora 14 will reach its end of life in less than a month, and no new packages are accepted for Fedora 14. (Updates to existing packages are possible until the end of life. At that point, Fedora 14 will be completely dead as far as the Fedora Project is concerned.) > Kevin, please take ownership of review BZs. Thanks! Whoops, fixed.
(PS: Minh, next time, when you file a "New Package SCM Request", please set the fedora-cvs flag to '?', not '+'. It gets set to '+' when the request is handled, as was done now with comment #41. I fixed that for you.)
> 3505780 tagBuild (noarch): free > 3505780 tagBuild (noarch): free -> open (x86-06.phx2.fedoraproject.org) > 3505780 tagBuild (noarch): open (x86-06.phx2.fedoraproject.org) -> FAILED: >ActionNotAllowed: tag requires admin permission > 0 free 1 open 3 done 1 failed >3505695 build (f16, /wicd-kde:8f92e20460e571c7b5a3b177dd7e695d8b7065a0): open >(x86-13.phx2.fedoraproject.org) -> FAILED: ActionNotAllowed: tag requires admin >permission > 0 free 0 open 3 done 2 failed >3505695 build (f16, /wicd-kde:8f92e20460e571c7b5a3b177dd7e695d8b7065a0) failed http://koji.fedoraproject.org/koji/taskinfo?taskID=3505695
Your fedpkg must be outdated. The build target "f16" hasn't been used since Fedora 16 got branched, and it's even released now. Fedora 16 builds should be built in "f16-candidate", Rawhide builds in "dist-rawhide". Normally, fedpkg gets that right for you, so make sure your fedpkg package is up to date. For this build, Rex Dieter fixed the tagging mess for you, but please make sure to use an up-to-date fedpkg when submitting more builds.
wicd-kde-0.2.3-3.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/wicd-kde-0.2.3-3.fc16
wicd-kde-0.2.3-3.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/wicd-kde-0.2.3-3.fc15
wicd-kde-0.2.3-3.fc16 has been pushed to the Fedora 16 testing repository.
wicd-kde-0.2.3-3.fc16 has been pushed to the Fedora 16 stable repository.
wicd-kde-0.2.3-3.fc15 has been pushed to the Fedora 15 stable repository.
wicd-kde-0.3.0-1.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/wicd-kde-0.3.0-1.fc16
wicd-kde-0.3.0-1.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/wicd-kde-0.3.0-1.fc15
wicd-kde-0.3.0-1.fc16 has been pushed to the Fedora 16 stable repository. If problems still persist, please make note of it in this bug report.
wicd-kde-0.3.0-1.fc15 has been pushed to the Fedora 15 stable repository. If problems still persist, please make note of it in this bug report.
wicd-kde-0.3.0-3.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/wicd-kde-0.3.0-3.fc17
wicd-kde-0.3.0-4.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/wicd-kde-0.3.0-4.fc17
Please do not reference the review request in all your updates! The review request is already resolved when the INITIAL "newpackage" update goes through. Further updates should NOT reference it.
wicd-kde-0.3.0-4.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report.