Bug 773470

I meant to post a link to my cinnamon review

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

<drive-by comment> If muffin has been changed enough that it *can't* work with gnome-shell or Moblin, that part of the description should be removed.

Reviewing this. As per Bill's comment -- I take it Cinnamon will require Muffin, but would Muffin still be a drop-in replacement for GNOME Shell? I'll check once the SRPM finish rebuilding.

Most of the package looks good. Things that need to be changed:

- please update description to say muffin is meant to be used as part of Cinnamon
- provide instructions to generate the tarball (see full review below)
- query upstream about their tagging policy (the tag in git should perhaps be
  3.2.1 not 1.0.0). Include link to the issue as a comment to the source
- update GConf schema scriptlet; there are now macros replacing the old shell
  script invocations


#+TODO: TODO(t) WAIT(w@/!) FAIL(f@) | DONE(d) N/A(n)

* TODO Review [66%]
  - [X] Names [2/2]
    - [X] Package name
    - [X] Spec name
  - [-] Package version [1/2]
	http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Versioning
    - [X] Version number
	  http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Version_Tag

          matches version in source code

    - [ ] Release tag
	  http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Release_Tag
	  http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages

          The last revision on the git repo has been tagged, bizarrely, 1.0.0 .
          If this is what you use to generate the tarball, then you can use the
          release naming scheme; if not you'd have to use the pre-release one
          (and encode the commit checksum in the release tag field)

          Please ask upstream about their tagging policy, and if possible ask
          them to rename the 1.0.0 tag to 3.2.1? That would avoid confusion in
          the future. If you file an issue on the Github issue tracker about this,
          just put a comment above the Source line with the summary and URL
  - [X] Meets [[http://fedoraproject.org/wiki/Packaging/Guidelines][guidelines]]
  - [ ] Source files match upstream
        Please generate the archive in a reproducible way: e.g.
        git clone git://github.com/linuxmint/muffin.git
        (cd muffin && git archive --format=tar --prefix=muffin-%%{version}/ \
         1.0.0 \ <-- this really should be %%{version} but now the tag is 1.0.0
        ) | xz -9 > muffin-%%{version}.tar.xz

        you can use bz2 as well but xz is preferred, esp by GNOME upstream

  - [X] [[http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries][No bundled libraries]]
        I don't see any
  - [X] License [4/4]
    - [X] License is Fedora-approved
    - [X] No licensing conflict
    - [X] License field accurate
    - [X] License included iff packaged by upstream
  - [-] rpmlint [1/2]
    - [ ] on src.rpm
      (filtering out bogus spelling errors)
          
      muffin.src: W: no-url-tag
      muffin.src:8: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 8)
      muffin.src: W: invalid-url Source0: muffin-3.2.1.tar.xz

      source issue is discussed above. for spaces and tab, decide
      which you want to use for indenting for using spaces, if you use
      Emacs, Ctrl-X H Esc-X untabify will convert tabs to spaces
      throughout the file
	    
    - [X] on x86_64.rpm
      (filtering out issues that are identical to SRPM)

      -debuginfo and -devel have many incorrect-fsf-address errors; this is
      harmless

      These are the real warnings;
      muffin.x86_64: W: shared-lib-calls-exit /usr/lib64/libmuffin.so.0.0.0 exit.5
      muffin.x86_64: W: non-conffile-in-etc /etc/gconf/schemas/muffin.schemas

      Both are also present in mutter; I'd ignore the first, and the second is
      a bit misleading as schemas are not really configuration files.

      So there is no real problem in the generated binaries.

  - [X] Language & locale [3/3]
    - [X] Spec in US English
    - [X] Spec legible
    - [X] Use %find_lang to handle locale files
  - [X] Build [3/3]
    - [X] Koji results
      http://koji.fedoraproject.org/koji/taskinfo?taskID=3754662
    - [X] BRs complete
    - [X] Directory ownership
  - [X] Spec inspection [8/8]
    - [X] ldconfig for libraries
    - [X] No duplicate files
    - [X] File permissions
    - [X] Filenames must be UTF-8
    - [X] no BuildRoot ([[https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag][except if targeting RHEL5]])
    - [X] Macro usage consistent
    - [X] Documentation [1/1]
      - [X] %doc files are non-essential
    - [X] Development [4/4]
      - [X] Headers in -devel
      - [X] If versioned .so's, unversioned in -devel
      - [X] -devel, -static requires main
      - [X] No .la
  - [X] Desktop file validation
  - [ ] [[http://fedoraproject.org/wiki/Packaging/ScriptletSnippets][Scriptlets]] [0/4]
    - [ ] GConf
      Please migrate to newer GConf-handling macros; they are easier to
      maintain:

      http://fedoraproject.org/wiki/ScriptletSnippets#GConf

Thank you for reviewing muffin, here's the update

SPEC: http://leigh123linux.fedorapeople.org/pub/review/muffin/2/muffin.spec


SRPM: http://leigh123linux.fedorapeople.org/pub/review/muffin/2/muffin-1.0.0-2.fc16.src.rpm

APPROVED. Changes look good.

* TODO Review [100%]
  - [X] Names [2/2]
    - [X] Package name
    - [X] Spec name
  - [X] Package version [2/2]
        http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Versioning
    - [X] Version number
          http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Version_Tag

          matches version in source code

    - [X] Release tag
          http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Release_Tag
          http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages
  - [X] Meets [[http://fedoraproject.org/wiki/Packaging/Guidelines][guidelines]]
  - [X] Source files match upstream
    ✗ sha1sum muffin-1.0.0.tar.gz ../SOURCES/muffin-1.0.0.tar.gz
    e5095a17c3ced6ae08133930a77e0490d0d6e756  muffin-1.0.0.tar.gz
    e5095a17c3ced6ae08133930a77e0490d0d6e756  ../SOURCES/muffin-1.0.0.tar.gz
  - [X] [[http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries][No bundled libraries]]
        I don't see any
  - [X] License [4/4]
    - [X] License is Fedora-approved
    - [X] No licensing conflict
    - [X] License field accurate
    - [X] License included iff packaged by upstream
  - [X] rpmlint [2/2]
    - [X] on src.rpm
      (filtering out bogus spelling errors)
      muffin.src: W: invalid-url Source0: muffin-1.0.0.tar.gz

      download URL does not have the name of the created file; just ignore
      the warning

    - [X] on x86_64.rpm
      (filtering out issues that are identical to SRPM)

      -debuginfo and -devel have many incorrect-fsf-address errors; this is
      harmless

      These are the real warnings;
      muffin.x86_64: W: shared-lib-calls-exit /usr/lib64/libmuffin.so.0.0.0 exit.5
      muffin.x86_64: W: non-conffile-in-etc /etc/gconf/schemas/muffin.schemas
      muffin.x86_64: W: dangerous-command-in-%pre rm
      muffin.x86_64: W: dangerous-command-in-%post rm

      The first two are also present in mutter; I'd ignore the first,
      and the second is a bit misleading as schemas are not really
      configuration files.

      The latter two are from the %gconf macros, so those are fine.
  - [X] Language & locale [3/3]
    - [X] Spec in US English
    - [X] Spec legible
    - [X] Use %find_lang to handle locale files
  - [X] Build [3/3]
    - [X] Koji results
      http://koji.fedoraproject.org/koji/taskinfo?taskID=3762346
    - [X] BRs complete
    - [X] Directory ownership
  - [X] Spec inspection [8/8]
    - [X] ldconfig for libraries
    - [X] No duplicate files
    - [X] File permissions
    - [X] Filenames must be UTF-8
    - [X] no BuildRoot ([[https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag][except if targeting RHEL5]])
    - [X] Macro usage consistent
    - [X] Documentation [1/1]
      - [X] %doc files are non-essential
    - [X] Development [4/4]
      - [X] Headers in -devel
      - [X] If versioned .so's, unversioned in -devel
      - [X] -devel, -static requires main
      - [X] No .la
  - [X] Desktop file validation
  - [X] [[http://fedoraproject.org/wiki/Packaging/ScriptletSnippets][Scriptlets]] [1/1]
    - [X] GConf
      http://fedoraproject.org/wiki/ScriptletSnippets#GConf

Thank you for the review.

New Package SCM Request
=======================
Package Name: muffin
Short Description: Window and compositing manager based on Clutter
Owners: leigh123linux
Branches: f16
InitialCC:

(In reply to comment #4)

> - query upstream about their tagging policy (the tag in git should perhaps be
>   3.2.1 not 1.0.0).



Done

https://github.com/linuxmint/muffin/issues/5

(In reply to comment #8)
> (In reply to comment #4)
> 
> > - query upstream about their tagging policy (the tag in git should perhaps be
> >   3.2.1 not 1.0.0).
> 
> Done
> 
> https://github.com/linuxmint/muffin/issues/5

Thanks. At least going from 1.0.0 back to 3.2.1 is easier than the other way around (which would involve bumping epochs!). Since Cinnamon is itself version 1.x I have a feeling that upstream plans to eventually renumber the versions down, but it never hurts to ask (and nag them to fix their configure.ac declarations).

I'm re-adding the blocker for cinnamon -- it's really for book-keeping and to avoid confusion (cf. Rahul's question on the cinnamon review discussion)

Git done (by process-git-requests).

Leigh, for your next package, please make your initial "update" reference the review request, so the request gets closed automatically once the package hits stable. Thanks!

Package Change Request
======================
Package Name: muffin
New Branches: epel7
Owners: leigh123linux

No epel7 branches yet.

Package Change Request
======================
Package Name: muffin
New Branches: epel7
Owners: leigh123linux

Git done (by process-git-requests).