[Bug 814887] Review Request: encuentro - Content visualization of the Canal Encuentro.

[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.


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

Toshio Ernie Kuratomi <a.badger@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #11 from Toshio Ernie Kuratomi <a.badger@xxxxxxxxx> 2012-04-25 22:13:15 EDT ---
NEEDSWORK

Good:
* This is an application package.  It is named by the application name
* The spec file is the package name.
* License is GPLv3 in spec file and sources which is an approved free software
license
* Spec file is mostly legible.  See the notes in Cosmetic, below
* Source matches upstream and upstream source url is the correct place
* No locale files so no need to mark them as lang files
* Not a shared elf library
* No bundled libraries
* Not designed to be relocatable
* Macros used consistently
* Code, not content
* No large documentation files
* Nothing in %doc affects runtime
* No scriptlets needed

Mustfix:
* Doesn't build because setup.py install doesn't install all of the files
listed in %files
* Doesn't build because __python is used instead of %{__python}
* Need to separate %build and %install sections.  Should look like this:
  %build
  %{__python} setup.py build

  %install
  %{__python} setup.py install --skip-build --root $RPM_BUILD_ROOT
* Documentation files need to be added in %files at least:
  %doc COPYING README.txt
  Since the target audience speaks Spanish, you probably want to include
  LEEMA.txt and AYUDA.txt as well.  AUTHORS is also nice to include
* Package creates %{_datadir}/encuentro/  Needs to own that directory in the
  %files section
* desktop-file-install needs to be used to install the desktop file


Cosmetic:
* Description (and changelog) lines should be no more than 80 characters.
* It's nicer to format multiple Requires just as in programming:
  Requires: python-mechanize, python-twisted, pygtk2, pyxdg
  or
  Requires: python-mechanize
  Requires: python-twisted
  Requires: pygtk2
  Requires: pyxdg
* There's no reason to give --record=INSTALLED_FILES to setup.py because we're
  not going to use that to construct our file list.  If you look, at the Python
  guidelines they say that --record isn't a good way to get the installed files
  because it does not record directories

Not yet done (Can't do these until the package builds):
* rpmlint
* builds in koji
* Test that packge runs
* Check %files vs what's installed again once the package builds
* Check file permissions
* Check that all filenames are utf-8

-- 
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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



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