Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=499069
Jens Petersen <petersen(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |fedora-haskell-list@redhat.
| |com, petersen(a)redhat.com
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=426753
Jens Petersen <petersen(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flag|fedora-review? |fedora-review+
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=426753
--- Comment #46 from Jens Petersen <petersen(a)redhat.com> 2009-05-05 00:00:45 EDT ---
Here is my formal review.
+:ok, !:needs attention, -:needs fixing, NA: not applicable
MUST Items:
[!] MUST: rpmlint output
Please fix the tabs that appeared spec file for the patch.
Also fixed in my next patch.
[+] MUST: Package Naming Guidelines
[+] MUST: spec file name must match base package %{name}
[+] MUST: Packaging Guidelines.
Follows Haskell Packaging Guidelines
[+] MUST: Licensing Guidelines
[+] MUST: License field in the package spec file must match actual license.
[+] MUST: include license files in %doc if available in source
[+] MUST: The spec file must be written in American English and be legible.
[+] MUST: source md5sum matches upstream release
03a8f0a420902d9eea3df1d8d62598c7 xmonad-0.8.1.tar.gz
[+] MUST: must successfully compile and build into binary rpms on one main arch
[+] MUST: if necessary use ExcludeArch for other archs
[+] MUST: All build dependencies must be listed in BuildRequires
[NA] MUST: use %find_lang macro for .po translations
[NA] MUST: packages which store shared library files in the dynamic linker's
default paths, must call ldconfig in %post and %postun.
[NA] MUST: If the package is designed to be relocatable, the packager must
state this fact in the request for review
[+] MUST: A package must own all directories that it creates.
[+] MUST: A package must not contain any duplicate files in the %files listing.
[+] MUST: Permissions on files must be set properly. Executables should be set
with executable permissions, for example.
[+] MUST: Each package must have a %clean section
[+] MUST: Each package must consistently use macros, as described in the macros
section of Packaging Guidelines.
[+] MUST: The package must contain code, or permissable content.
[+] MUST: Large documentation files should go in a doc subpackage.
[+] MUST: If a package includes something as %doc, it must not affect the
runtime of the application.
[+] MUST: Header files must be in a -devel package.
[NA] MUST: Static libraries must be in a -static package.
[NA] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
(for directory ownership and usability).
[NA] MUST: If a package contains library files with a suffix (e.g.
libfoo.so.1.1), then library files that end in .so (without suffix) must go in
a -devel package.
[NA] MUST: In the vast majority of cases, devel packages must require the base
package using a fully versioned dependency: Requires: %{name} =
%{version}-%{release}
[NA] MUST: Packages must NOT contain any .la libtool archives, these should be
removed in the spec.
[+] MUST: Packages containing GUI applications must include a %{name}.desktop
file, and that file must be properly installed with desktop-file-install in the
%install section.
[+] MUST: Packages must not own files or directories already owned by other
packages.
[+] MUST: At the beginning of %install, each package MUST run rm -rf
%{buildroot} (or $RPM_BUILD_ROOT).
[+] MUST: All filenames in rpm packages must be valid UTF-8.
SHOULD Items:
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The package should compile and build into binary rpms on all
supported architectures.
[+] SHOULD: The reviewer should test that the package functions as described.
[+] SHOULD: If scriptlets are used, those scriptlets must be sane.
Package is APPROVED.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=426753
--- Comment #45 from Jens Petersen <petersen(a)redhat.com> 2009-05-04 23:58:51 EDT ---
I tested the scratch build on f11 with f12 ghc and it works fine for me.
I will attach a patch for the suggestions above after this.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=426753
Jens Petersen <petersen(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flag|needinfo?(petersen(a)redhat.c |
|om) |
--- Comment #44 from Jens Petersen <petersen(a)redhat.com> 2009-05-04 00:03:40 EDT ---
(In reply to comment #43)
> New update
Thanks!
> 1) carries around a ppc workaround that really should go in macros.ghc
Yeah we could add something in the macros to help with this, but it is a bit
tedious: I hope we can fix the real problem on ppc soon.
> 2) only builds on fedora 12 (yay?)
I hope to backport latest ghc to f11 soon. We might have to wait for a new
gtk2hs release.
> 3) converts the value added default config into a patch directly against the
> upstream source. what this patch does is replaces the xmonad line of the
> default verbose config with an expansion that loads xterm with the man page
> showing. in the future, i'll probably add a comment to the user
Good idea.
I think better to rename the patch to xmonad-config-show-manpage.patch
or something like that.
> 4) includes just xmonad-start without doing xmonad-session.
Good - I think the name in the .desktop file should just be "xmonad" or
"XMonad" though not "xmonad-start".
> 5) renumbers the source to make more sense
> 6) includes other changes proposed here
Ok
Otherwise looks pretty good to me now. I am going to take it for a spin and
then do the review.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.