Spec URL: http://jamesni.fedorapeople.org/fonts-tweak-tool/fonts-tweak-tool.spec SRPM URL: http://jamesni.fedorapeople.org/fonts-tweak-tool/fonts-tweak-tool-0.0.1-1.fc16.src.rpm Description: fonts-tweak-tool is a GUI tool for customizing fonts per language on desktops using fontconfig.
I will review this package.
Some rpmlint issues: --- rpmlint -i /home/mohamed/rpmbuild/RPMS/noarch/fonts-tweak-tool-0.0.1-1.fc16.noarch.rpm fonts-tweak-tool.noarch: W: spelling-error %description -l en_US fontconfig -> configuration The value of this tag appears to be misspelled. Please double-check. fonts-tweak-tool.noarch: E: description-line-too-long C fonts-tweak-tool is a GUI tool for customizing fonts per language on desktops using fontconfig. Your description lines must not exceed 80 characters. If a line is exceeding this number, cut it to fit in two lines. fonts-tweak-tool.noarch: W: invalid-url URL: https://github.com/downloads/jamesni/fonts-tweak-tool HTTP Error 404: Not Found The value should be a valid, public HTTP, HTTPS, or FTP URL. fonts-tweak-tool.noarch: E: incorrect-fsf-address /usr/bin/fonts-tweak-tool The Free Software Foundation address in this file seems to be outdated or misspelled. Ask upstream to update the address, or if this is a license file, possibly the entire file with a new copy available from the FSF. fonts-tweak-tool.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/fontstweak/tool.py The Free Software Foundation address in this file seems to be outdated or misspelled. Ask upstream to update the address, or if this is a license file, possibly the entire file with a new copy available from the FSF. fonts-tweak-tool.noarch: W: no-manual-page-for-binary fonts-tweak-tool Each executable in standard binary directories should have a man page. 1 packages and 0 specfiles checked; 3 errors, 3 warnings. --- You can ignore the spelling issues, they are false positive. You can also skip the missing man page issue, it's not mandatory to have one. But the other warnings/errors must be solved: - I suppose the correct URL is https://github.com/jamesni/fonts-tweak-tool - since you are the developer of font-tweak-tools, you should really fix the FSF address in the comments of your source code Now about the .spec file: - you don't need anymore to set the %python_sitelib macro, its definition is now provided by the rpm package: http://fedoraproject.org/wiki/Packaging:Python#Macros - the summary is maybe too mysterious: something like " Tool for customizing fonts per language" may be more explicit, or any other description, at your convenience - the Group tag "Development/Tools" is maybe far from corresponding to your tool. Something like "User Interface/Desktops" is more appropriate. - You could really take advantage of macros in the Source0 URL: Source0: https://github.com/downloads/jamesni/fonts-tweak-tool/%{name}-%{version}.tar.gz - The BuildRoot tag is not needed anymore, as well as cleaning the build root (occurrences "rm -rf $RPM_BUILD_ROOT" can be dropped): http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag - I suggest you to replace "python-setuptools" by "python2-setuptools" in your BuildRequires, in case Fedora switches to Python 3 by default. - I don't think the version condition on libeasyfc is necessary, since only Fedora 16 and Rawhide provide libeasyfc, at version 0.5. - The %defattr macro is useless now in %files: http://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions - The %clean target is useless too: http://fedoraproject.org/wiki/Packaging:Guidelines#.25clean
(In reply to comment #2) > - I don't think the version condition on libeasyfc is necessary, since only > Fedora 16 and Rawhide provide libeasyfc, at version 0.5. Rawhide has version 0.6, in fact. But it doesn't change my advice about the version ;) By the way, since your package seems to require pygobject3 and gtk3, you must add them as Requires in your .spec file. Moreover, since fonts-tweak-tool is a graphical tool, you must provide a .desktop file: http://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files If you need an icon for your project, you could request the Design team (http://fedoraproject.org/wiki/Design#What_We_Do) to draw one: https://fedorahosted.org/design-team/newticket I tried to test your application, but I get this error under Fedora 16: --- $ fonts-tweak-tool ERROR:root:Could not find any typelib for Easyfc Traceback (most recent call last): File "/usr/bin/fonts-tweak-tool", line 23, in <module> from fontstweak import tool File "/usr/lib/python2.7/site-packages/fontstweak/tool.py", line 30, in <module> from gi.repository import Easyfc ImportError: cannot import name Easyfc --- The libeasyfc package is obviously installed (v. 0.5), as well as libeasyfc-gobject
Hi Morabity Thanks a lot for your comments, I just update the spec file and source code base on your comment: Spec URL: http://jamesni.fedorapeople.org/fonts-tweak-tool/fonts-tweak-tool.spec SRPM URL: http://jamesni.fedorapeople.org/fonts-tweak-tool/fonts-tweak-tool-0.0.2-1.fc16.src.rpm Basically, I update the licenses file to LGPLv3+, as well as the header of source files. The spec file is modified based on your comment. I also add a desktop file in package following the guideline you provided. I also try rpmlint on my side, it seems everything works fine. I didn't meet the errors you meet, i install the rpm in a new installed Fedora16 and then run "yum install libeasyfc*" to install all the packages of libeasyfc, after that i run fonts-tweak-tool, the program works as expected. So could you try again? Best Regards James
(In reply to comment #4) > Hi Morabity My first name is Mohamed ;) > I didn't meet the errors you meet, i install the rpm in a new installed > Fedora16 and then run "yum install libeasyfc*" to install all the packages of > libeasyfc, after that i run fonts-tweak-tool, the program works as expected. > > So could you try again? I got the problem: GObject introspection relies on *.typelib files installed in /usr/lib*/girepository-1.0/. For libeasyfc, these files are provided by the libeasyfc-gobject-devel file, whereas they have nothing to do with development. We can't seriously require *-devel package to run such an application. As a result, I filled a bug report on libeasyfc to ask its maintained to move these files in the libeasyfc-gobject package. Once this bug is solved, you will require the libeasyfc-gobject package instead of the libeasyfc one. Please replace also BuildRequires: python3-setuptools by: BuildRequires: python2-setuptools fonts-tweak-tool is written for Python 2 and built using Python 2, until you switch to Python 3 ;). Once these two issues above solved, I will approve this package.
One more comment: %install %{__python} setup.py install --skip-build --root $RPM_BUILD_ROOT desktop-file-install --dir=%{buildroot}%{_datadir}/applications fonts-tweak-tool.desktop You should use $RPM_BUILD_ROOT *or* %{buildroot} in all your .spec file, not the two styles at the same time: http://fedoraproject.org/wiki/Packaging:Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS About the license change: there's no problem here to switch to GPL to LGPL, but I just wonder what motivated such a radical choice.
Hi Mohamed, I didn't find python2-setuptools, there are only python3-setuptools and python-setuptools, so i guess i should change it back to python-setuptools?
(In reply to comment #7) > I didn't find python2-setuptools, there are only python3-setuptools and > python-setuptools, so i guess i should change it back to python-setuptools? Oops, I'm really sorry, I was wrong, indeed. Only the python package provides the "python2" feature, it doesn't concern other Python packages. So, yes, you can set python-setuptools as before.
Hi Mohamed, Thanks a lot, i have fix issues of spec file: - Replace libeasyfc with libeasyfc-gobject - Require python-setuptools - Replace $RPM_BUILD_ROOT with %{buildroot} Spec URL: http://jamesni.fedorapeople.org/fonts-tweak-tool/fonts-tweak-tool.spec SRPM URL: http://jamesni.fedorapeople.org/fonts-tweak-tool/fonts-tweak-tool-0.0.2-2.fc16.src.rpm I saw tagoh have fixed bug 788112 and commit a new package.
Hi Mohamed, Have you see the new package i created last week? Do you think it is OK to pass the review? Best Regards James
Sorry for this late answer, I got quite busy this week. Here is the review: Package Review ============== Key: - = N/A x = Pass ! = Fail ? = Not evaluated ==== Generic ==== [x]: MUST Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: MUST Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: MUST All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: MUST Buildroot is not present Note: Unless packager wants to package for EPEL5 this is fine [x]: MUST Package contains no bundled libraries. [x]: MUST Changelog in prescribed format. [x]: MUST Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) Note: Clean would be needed if support for EPEL is required [x]: MUST Sources contain only permissible code or content. [x]: MUST Each %files section contains %defattr if rpm < 4.4 Note: Note: defattr macros not found. They would be needed for EPEL5 [x]: MUST Macros in Summary, %description expandable at SRPM build time. [x]: MUST Package contains a properly installed %{name}.desktop using desktop- file-install file if it is a GUI application. [x]: MUST Package requires other packages for directories it uses. [x]: MUST Package uses nothing in %doc for runtime. [x]: MUST Package is not known to require ExcludeArch. [x]: MUST Permissions on files are set properly. [x]: MUST Package does not contain duplicates in %files. [x]: MUST Spec file lacks Packager, Vendor, PreReq tags. [x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. Note: rm -rf would be needed if support for EPEL5 is required [x]: MUST 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]: MUST License field in the package spec file matches the actual license. [!]: MUST Package consistently uses macros (instead of hard-coded directory names). [x]: MUST Package meets the Packaging Guidelines. [x]: MUST Package is named according to the Package Naming Guidelines. [x]: MUST Package does not generates any conflict. [x]: MUST Package obeys FHS, except libexecdir and /usr/target. [x]: MUST Package must own all directories that it creates. [x]: MUST Package does not own files or directories owned by other packages. [x]: MUST Package installs properly. [x]: MUST Requires correct, justified where necessary. [!]: MUST Rpmlint output is silent. rpmlint fonts-tweak-tool-0.0.2-2.fc18.src.rpm fonts-tweak-tool.src: W: spelling-error %description -l en_US fontconfig -> configuration 1 packages and 0 specfiles checked; 0 errors, 1 warnings. rpmlint fonts-tweak-tool-0.0.2-2.fc18.noarch.rpm fonts-tweak-tool.noarch: E: explicit-lib-dependency libeasyfc-gobject fonts-tweak-tool.noarch: W: spelling-error %description -l en_US fontconfig -> configuration fonts-tweak-tool.noarch: W: no-manual-page-for-binary fonts-tweak-tool 1 packages and 0 specfiles checked; 1 errors, 2 warnings. [x]: MUST Sources used to build the package match the upstream source, as provided in the spec URL. [x]: MUST Spec file is legible and written in American English. [x]: MUST Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: MUST Package contains a SysV-style init script if in need of one. [x]: MUST File names are valid UTF-8. [x]: SHOULD Reviewer should test that the package builds in mock. [x]: SHOULD If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: SHOULD Dist tag is present. [x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q --requires). [x]: SHOULD Package functions as described. [x]: SHOULD Package does not include license text files separate from upstream. [x]: SHOULD SourceX is a working URL. [ ]: SHOULD Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: SHOULD Package should compile and build into binary rpms on all supported architectures. [x]: SHOULD %check is present and all tests pass. [x]: SHOULD Packages should try to preserve timestamps of original installed files. [x]: SHOULD Spec use %global instead of %define. Issues: [!]: MUST Rpmlint output is silent. rpmlint fonts-tweak-tool-0.0.2-2.fc18.src.rpm fonts-tweak-tool.src: W: spelling-error %description -l en_US fontconfig -> configuration 1 packages and 0 specfiles checked; 0 errors, 1 warnings. rpmlint fonts-tweak-tool-0.0.2-2.fc18.noarch.rpm fonts-tweak-tool.noarch: E: explicit-lib-dependency libeasyfc-gobject fonts-tweak-tool.noarch: W: spelling-error %description -l en_US fontconfig -> configuration fonts-tweak-tool.noarch: W: no-manual-page-for-binary fonts-tweak-tool 1 packages and 0 specfiles checked; 1 errors, 2 warnings. >>> As said above, this can be ignored safely. [!]: MUST Package consistently uses macros (instead of hard-coded directory names). >>> You should use %{name} instead of "fonts-tweak-tool" in this line: desktop-file-install --dir=%{buildroot}%{_datadir}/applications fonts-tweak-tool.desktop By the way, since you use the preferences-system icon in your .desktop file, I suggest you to add also the hicolor-icon-theme package as Requires. Once the above issues fixed, I will approve the package.
Hi Mohamed, Thanks a lot for your review, i have fix these two issues you mentioned, please review the new Spec file and SRPM: Spec URL: http://jamesni.fedorapeople.org/fonts-tweak-tool/fonts-tweak-tool.spec SRPM URL: http://jamesni.fedorapeople.org/fonts-tweak-tool/fonts-tweak-tool-0.0.2-3.fc16.src.rpm Best Regards
This package is APPROVED! Thanks for your work on this great application!
New Package SCM Request ======================= Package Name: fonts-tweak-tool Short Description: a GUI tool for customizing fonts per language on desktop. Owners: jamesni Branches: f16 f17 InitialCC: tagoh petersen pwu i18n-team
Git done (by process-git-requests).
fonts-tweak-tool-0.0.2-3.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/fonts-tweak-tool-0.0.2-3.fc16
fonts-tweak-tool-0.0.2-3.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/fonts-tweak-tool-0.0.2-3.fc17
fonts-tweak-tool-0.0.3-1.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/fonts-tweak-tool-0.0.3-1.fc17
fonts-tweak-tool-0.0.3-1.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/fonts-tweak-tool-0.0.3-1.fc16
fonts-tweak-tool-0.0.3-1.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report.
fonts-tweak-tool-0.0.3-1.fc16 has been pushed to the Fedora 16 stable repository. If problems still persist, please make note of it in this bug report.