Bug 468633 (wput)
Summary: | Review Request: wput - A utility for uploading files or whole directories to remote ftp-servers | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Itamar Reis Peixoto <itamar> |
Component: | Package Review | Assignee: | Fabian Affolter <mail> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, mail, notting |
Target Milestone: | --- | Flags: | mail:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2008-11-19 14:46:39 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Itamar Reis Peixoto
2008-10-27 00:06:58 UTC
Just some small comments on your spec file - Release: 1.0%{?dist} - Just '1' is enough. Next release will be '2' - You can use the macro %{name} everywhere where you used 'wput' - %files section - '%defattr(-,root,root)', the usual one is '%defattr(-,root,root,-)' https://fedoraproject.org/wiki/PackageMaintainers/CreatingPackageHowTo#.25files_section - Is it not enough just to use '%{_bindir}/%{name}' insteed of '%attr(0755, root, root) %{_bindir}/wput' ? please look again http://ispbrasil.com.br/wput/wput.spec http://ispbrasil.com.br/wput/wput-0.6.1-2.fc9.src.rpm If this is your first package you need to seek a sponsor. I can't sponsor you but I can do the review. Please visit the following pages: https://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored https://fedoraproject.org/wiki/PackageMaintainers/Join#Create_Your_Review_Request (-> FE-NEEDSPONSOR ;-) ) Again some comments on your spec file - Source: - Please use 'Source0: http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz' instead of a link to a mirror https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net - Patch1: wput-destdir.patch - The first patch is 'Patch0' - %changelog - To every release bump belongs an changelog entry. https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs (In reply to comment #3) > (-> FE-NEEDSPONSOR ;-) ) Forget this...you added the blocker already. fixed Can I start bumping versions later ? http://ispbrasil.com.br/wput/wput.spec http://ispbrasil.com.br/wput/wput-0.6.1-3.fc9.src.rpm > The first patch is 'Patch0'
Just for the record: that's more a cosmetic / pedantic problem. rpm is very
happy to use anything, as long as the definition of the patch (the PatchN line)
and it's usage (the %PatchN line) correspond.
But yes, for a first submission looking nice (from a cosmetic point of view) is
important and could be a decision factor.
More important: please preserve the older changelog entries when you/add make
changes and create new releases of the spec. I am looking at releases 2 and 3 right now and I would have liked to know what was changed compared to the first ones. Especially as the _current_ changelog (the one from release 3) still says "Initial package."
A correct changelog would have probably had something along:
* Thu Oct 27 2008 Itamar Reis Peixoto <itamar.br> - 0.6.1-3
- start counting patches from 0
* Thu Oct 27 2008 Itamar Reis Peixoto <itamar.br> - 0.6.1-2
- fixes %%files, consistent use of macros<, other relevant changes, if any>
* Thu Oct 26 2008 Itamar Reis Peixoto <itamar.br> - 0.6.1-1.0
- Initial package.
(In reply to comment #6) > Can I start bumping versions later ? I'm not talking about the version of the source. The 'Release' is for the spec file. Again changelog...in your lasted spec file is only *ONE* changelog entry. If your Release is '3' there have to be '3' entries. Your spec file should contain. For the reviewers it's much easier to keep track of the changes. %changelog * Mon Oct 27 2008 Itamar Reis Peixoto <itamar.br> - 0.6.1-3 - Fix Source0 - Rename Patch1 to Patch0 * Mon Oct 27 2008 Itamar Reis Peixoto <itamar.br> - 0.6.1-2 - Changes acc. #468633 Comment #1 * Thu Oct 26 2008 Itamar Reis Peixoto <itamar.br> - 0.6.1-1 - Initial package. Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=908019 My nose smells missing BR: gnutls-devel. And I have a feeling that gettext is needed, too , despite the fact that the locale files get created (the configure script complains about missing gettext) gettext is definitely missing and adding gnutls will be a good idea ;-) BuildRequires: gettext BuildRequires: gnutls-devel Itamar, take a look into the log at http://koji.fedoraproject.org/koji/getfile?taskID=908022&name=build.log (In reply to comment #11) > gettext is definitely missing and adding gnutls will be a good idea ;-) > > BuildRequires: gettext > BuildRequires: gnutls-devel > Done http://ispbrasil.com.br/wput/wput.spec http://ispbrasil.com.br/wput/wput-0.6.1-4.fc9.src.rpm Thanks for the changes. I will make a full review today. (Removing NEEDSPONSOR) Package Review ============== Key: - = N/A x = Check ! = Problem ? = Not evaluated === REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines. [x] Spec file name must match the base package %{name}, in the format %{name}.spec. [x] Package meets the Packaging Guidelines. [x] Package successfully compiles and builds into binary RPMs on at least one supported architecture. Tested on: F9/i386 [x] Rpmlint output: Source RPM: [fab@laptop024 SRPMS]$ rpmlint wput* 1 packages and 1 specfiles checked; 0 errors, 0 warnings. Binary RPMs: [fab@laptop024 i386]$ rpmlint -i wput* 2 packages and 0 specfiles checked; 0 errors, 0 warnings. [x] Package is not relocatable. [x] Buildroot is correct master : %{_tmppath}/%{name}-%{version}-%{release}-root spec file: %{_tmppath}/%{name}-%{version}-%{release}-root [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x] License field in the package spec file matches the actual license. License type: GPLv2+ [x] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x] Spec file is legible and written in American English. [x] Sources used to build the package matches the upstream source, as provided in the spec URL. SHA1SUM upstream: 67125acab1d520e5d2a0429cd9cf7fc379987f30d5bbed0b0e97b92b554fcc13 wput-0.6.1.tgz SHA1SUM of Source RPM: 67125acab1d520e5d2a0429cd9cf7fc379987f30d5bbed0b0e97b92b554fcc13 wput-0.6.1.tgz [x] Package is not known to require ExcludeArch [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x] The spec file handles locales properly. [-] ldconfig called in %post and %postun if required. [x] Package must own all directories that it creates. [x] Package requires other packages for directories it uses. [x] Package does not contain duplicates in %files. [x] Permissions on files are set properly. [x] Package has a %clean section, which contains rm -rf %{buildroot}. [x] Package consistently uses macros. [x] Package contains code, or permissable content. [-] Large documentation files are in a -doc subpackage, if required. [x] Package uses nothing in %doc for runtime. [-] Header files in -devel subpackage, if present. [-] Static libraries in -devel subpackage, if present. [-] Package requires pkgconfig, if .pc files are present. [-] Development .so files in -devel subpackage, if present. [-] Fully versioned dependency in subpackages, if present. [-] Package does not contain any libtool archives (.la). [-] Package contains a properly installed %{name}.desktop file if it is a GUI application. [x] Package does not own files or directories owned by other packages. === SUGGESTED ITEMS === [x] Latest version is packaged. [x] Package does not include license text files separate from upstream. [-] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x] Reviewer should test that the package builds in mock. Tested on: F9/i386 [x] Package should compile and build into binary RPMs on all supported architectures. Tested F9: http://koji.fedoraproject.org/koji/taskinfo?taskID=910045 Tested F10: http://koji.fedoraproject.org/koji/taskinfo?taskID=910050 [?] Package functions as described. [-] Scriptlets must be sane, if used. [-] The placement of pkgconfig(.pc) files is correct. [-] File based requires are sane. I see no further blocker, package APPROVED New Package CVS Request ======================= Package Name: wput Short Description: A utility for uploading files or whole directories to remote ftp-servers Owners: itamarjp Branches: F-9 F-10 InitialCC: fabian cvs done. wput-0.6.1-4.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/wput-0.6.1-4.fc9 wput-0.6.1-4.fc9 has been pushed to the Fedora 9 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update wput'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-9330 wput-0.6.1-4.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report. Package Change Request ====================== Package Name: wput New Branches: EL-4 EL-5 Owners: itamarjp cvs done. |