[Bug 226452] Merge Review: system-config-bind

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: system-config-bind


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226452


kevin@xxxxxxxxx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|nobody@xxxxxxxxxxxxxxxxx    |kevin@xxxxxxxxx
               Flag|                            |fedora-review?




------- Additional Comments From kevin@xxxxxxxxx  2007-03-13 23:44 EST -------
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
See below - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
See below - License
See below - License field in spec matches
See below - License file included in package
OK - Spec in American English
OK - Spec is legible.
See below - Sources match upstream md5sum:
OK - BuildRequires correct
OK - Spec handles locales/find_lang
See below - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
See below - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.

OK - Package is a GUI app and has a .desktop file

OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
See below - final provides and requires are sane

SHOULD Items:

OK - Should build in mock.
OK - Should build on all supported archs
OK - Should function as described.
See below - Should have sane scriptlets.
OK - Should have subpackages require base package with fully versioned depend.
OK - Should have dist tag
OK - Should package latest version
10 outstanding bugs - check for outstanding bugs on package.

Issues:

1. You use $RPM_BUILD_ROOT and %{buildroot}. Pick one style and use that.

2. The URL seems to go to a 404 page:
URL:            http://people.redhat.com/~jvdias/system-config-bind

3. The spec says the license is GPL, but there's no mention of the
license in the contents of the tar.gz. Perhaps a COPYING file or a README
explaining that it's released under the GPL.

4. Since redhat/fedora is upstream for this package, can you add
a note as suggested in:
http://www.fedoraproject.org/wiki/Packaging/SourceURL#head-413e1c297803cfa9de0cc4c56f3ac384bff5dc9e

5. You have a desktop file, but aren't installing it with
desktop-file-install. Perhaps take a look at:
http://www.fedoraproject.org/wiki/PackagingDrafts/DesktopFiles

6. Is there a reason that every file in this package is in the
"bind" group? This results in 361 "non-standard-gid" warnings from rpmlint.

7. The python >= 2.2 Requires shouldn't be needed anymore.

8. %define debug_package %{nil} shouldn't be needed, this is a noarch package.

9. Is all the removing and linking in post and triggerun needed anymore?
How long ago was that version? Looks like aug 2005... perhaps we can remove it now?

10. rpmlint says:

a)
non-standard-gid (361 times)

Why does this need to be gid bind? Doesn't it run as root?

b)
E: system-config-bind invalid-desktopfile
/tmp/system-config-bind-4.0.2-3.fc7.noarch.rpm.8818/usr/s
hare/applications/system-config-bind.desktop

Suggest: Install desktop file correctly.

c)
E: system-config-bind no-cleaning-of-buildroot %install

Suggest: rm -rf $RPM_BUILD_ROOT at the top of install?

d)
E: system-config-bind non-standard-executable-perm
/usr/share/system-config-bind/BIND.py 0754
(repeated for every file in that directory)

Suggest: Permissions on those should be 0755 if they are scripts to be executed,
or more likey if they are just modules being included they should be 0644.

e)
E: system-config-bind obsolete-not-provided bindconf
E: system-config-bind obsolete-not-provided redhat-config-bind

Suggest: Do we need to keep these around still?

f)
E: system-config-bind script-without-shebang
/usr/share/system-config-bind/EditDialog.py
E: system-config-bind script-without-shebang
/usr/share/system-config-bind/NewZone.py
E: system-config-bind script-without-shebang
/usr/share/system-config-bind/system-config-bind.glade
E: system-config-bind script-without-shebang /usr/share/system-config-bind/View.py

Suggest: perms should be 0644?

g)
W: system-config-bind dangerous-command-in-%post rm
W: system-config-bind dangerous-command-in-%trigger rm

Suggest: can we just do away with that post and trigger stuff?

h)
W: system-config-bind macro-in-%changelog version

Suggest: Change the %version to %%version int he changelog. 

i) 
W: system-config-bind mixed-use-of-spaces-and-tabs (spaces: line 40, tab: line 1)

Suggest: minor. Could change to all spaces.

j)
W: system-config-bind no-dependency-on usermode
W: system-config-bind no-dependency-on usermode

Suggest: does there need to be a dependency on usermode here?

k)
W: system-config-bind non-conffile-in-etc /etc/pam.d/bindconf
W: system-config-bind non-conffile-in-etc /etc/pam.d/system-config-bind
W: system-config-bind non-conffile-in-etc /etc/security/console.apps/bindconf
W: system-config-bind non-conffile-in-etc
/etc/security/console.apps/system-config-bind

Suggest: Should any of those be marked %config? or %config(noreplace)?

l)
W: system-config-bind rpm-buildroot-usage %build rm -rf $RPM_BUILD_ROOT

Suggest: get rid of the rm -rf $RPM_BUILD_ROOT at the top of build.

m)
W: system-config-bind summary-ended-with-dot The Red Hat BIND DNS Configuration
Tool.

Suggest: remove . at end of summary. 

n)
W: system-config-bind symlink-should-be-relative
/usr/share/locale/ar/LC_MESSAGES/bindconf.mo /usr/
share/locale/ar/LC_MESSAGES/system-config-bind.mo

Suggest: Make those relative symlinks (works better in chroot type setups).

11. Might check the outstanding bugs and see if any can be cleaned up
as part of this review. In particular you might fix the typos mentioned in
232054


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@xxxxxxxxxx
http://www.redhat.com/mailman/listinfo/fedora-package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]