Bug 543549

$ gem list -b haml
returns that the latest is 2.2.15 and it seems to have been
released on 2009-12-01. Would you update first?

Done. Updated packages are at the same place (http://mogurakun.web.runbox.net/)

Some notes:

* %define -> %global
  - Now Fedora prefers to use %global over %define.
    https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define

* License
  - test/haml/spec/README.md is under WTFPL so the license tag
    should be "MIT and WTFPL".

* Requires
  - Please add the needed rubygem related dependency.
    For example, lib/haml/html.rb contains:
--------------------------------------------------------
    62  require 'hpricot'
--------------------------------------------------------
    So this package may need "Requires: rubygem(hpricot)" (here
    not speaking of BuildRequires).
    Note that I don't know if this dependency is optional or not.
    Also please check other dependency (if any).

* %check
  - I think
    * hardcoding test files as %test_files is not preferable.
      It is difficult to see what this file list came from.
    * also even if hardcoding these files is needed, defining %test_files
      is not needed because
      - %test_files is in essence used only in one place (in %check)
      - Whether adding executable permission to a script or not should
        be determined (for this case) by checking if the script has
        shebang or not, and should not be determined by hardcoded file list.

    I think
    - fixing Rakefile and execute "rake test"
    - or using
------------------------------------------------------------
pushd %{buildroot}%{geminstdir}
# The following -path list is from Rakefile
find * \
	-path 'test/*/*_test.rb' \
	-not -path 'test/rails/*' \
	-not -path 'test/plugins/*' \
	-not -path 'test/haml/spec/*' | \
	while read f
do
	ruby $f
done
------------------------------------------------------------
    is better.

* Macros
  - As %geminstdir is already defined, use the macro in %files.

* %changelog style
  - When using Fedora CVS system, it is convenient when you put one line
    between each %changelog entry (for "make clog", for example), like
------------------------------------------------------------
%changelog
* Wed Dec 02 2009 Michal Babej <mbabej> - 2.2.15-1
- Update to new upstream release

* Wed Dec 02 2009 Michal Babej <mbabej> - 2.2.14-1
- Initial package
-------------------------------------------------------------

By the way it is appreciated if you post the full URL of the new
spec/srpm.

(In reply to comment #3)
> * %define -> %global
>   - Now Fedora prefers to use %global over %define.

Fixed.

> 
> * License
>   - test/haml/spec/README.md is under WTFPL so the license tag
>     should be "MIT and WTFPL".

Fixed.

> 
> * Requires
>   - Please add the needed rubygem related dependency.
>     Note that I don't know if this dependency is optional or not.

/usr/bin/html2haml requires it, so i think it's not optional. Added.

>     Also please check other dependency (if any).
I checked all files for requires, and other possible external tools:

1. extra/haml-mode.el and extra/sass-mode.el are emacs highlighting files; not sure what to do with these.

2. extra/update_watch.rb requires sinatra and json, but this file seems only useful to haml developers, i think, so i'd like to remove it.

3. lib/haml/filters.rb: this depends on rubygem-RedCloth for Textile filter, but Markdown and Maruku filters doesn't have packaged dependencies in fedora (afaict). The code in filters.rb can handle it though, and these filters are not a requirement to use haml. What do you suggest ?

4. Rakefile has many dependencies (tlsmail, yard, rcov/rcovtask, ruby-prof, "git" command...) and i'm not sure how useful it is. The only thing i'd like to keep is the 'test' task.

5. test/benchmark has more requires (erubis, markaby, rbench - i didn't find this packaged)

7. test/haml/spec/lua_haml_spec.lua - requires lua
8. test/haml/spec/ruby_haml_test.rb - requires json; doesn't work currently, but i have a patch to make this work

6. test/sass/plugin_test.rb has require merb, but it will skip the test if no merb is found.

> 
> * %check
>       - Whether adding executable permission to a script or not should
>         be determined (for this case) by checking if the script has
>         shebang or not, and should not be determined by hardcoded file list.

Not all files from *_test.rb have shebang (Though i could create a patch for this and ask upstream to integrate it)

>     I think
>     - fixing Rakefile and execute "rake test"

I prefer this one (where by fixing i mean leave only "test" task)

> 
> * Macros
>   - As %geminstdir is already defined, use the macro in %files.

Fixed.

> 
> * %changelog style

Fixed.

> By the way it is appreciated if you post the full URL of the new
> spec/srpm.

Sure.
SRPM: http://mogurakun.web.runbox.net/rubygem-haml-2.2.15-2.fc12.src.rpm
Spec: http://mogurakun.web.runbox.net/rubygem-haml.spec

For -2:

(In reply to comment #4)
> (In reply to comment #3)
> > * Requires
> >   - Please add the needed rubygem related dependency.
> >     Note that I don't know if this dependency is optional or not.
> 
> 3. lib/haml/filters.rb: this depends on rubygem-RedCloth for Textile filter,
> but Markdown and Maruku filters doesn't have packaged dependencies in fedora
> (afaict). The code in filters.rb can handle it though, and these filters are
> not a requirement to use haml. What do you suggest ?

  - It seems filters related dependency is optional, so I don't think
    adding "R: rubygem(RedCloth)" is strictly needed.

> 4. Rakefile has many dependencies (tlsmail, yard, rcov/rcovtask, ruby-prof,
> "git" command...) and i'm not sure how useful it is. The only thing i'd like to
> keep is the 'test' task.

  - We usually don't add dependency for Rakefile based dependency
    ( By the way I like to create -doc subpackage for rubygem based rpm
      and I usually put Rakefile to -doc, not to main package )

> > * %check
> >       - Whether adding executable permission to a script or not should
> >         be determined (for this case) by checking if the script has
> >         shebang or not, and should not be determined by hardcoded file list.
> 
> Not all files from *_test.rb have shebang (Though i could create a patch for
> this and ask upstream to integrate it)

  - But with your current rpm only ruby script with shebang have executable
    permission (and not all *_test.rb have shebang) anyway, so judging by
    my method should be possible.
    ( By the way if scripts without shebang have executable permission, or
      if scripts with shebang don't have exectable permission, rpmlint
      warns about this ).

    The reason I am talking about this is that I think hardcoding %test_files
    list should be avoided unless impossible.

(In reply to comment #5)
> 
>   - It seems filters related dependency is optional, so I don't think
>     adding "R: rubygem(RedCloth)" is strictly needed.

OK.

> 
> > 4. Rakefile has many dependencies (tlsmail, yard, rcov/rcovtask, ruby-prof,
> > "git" command...) and i'm not sure how useful it is. The only thing i'd like to
> > keep is the 'test' task.
> 
>   - We usually don't add dependency for Rakefile based dependency
>     ( By the way I like to create -doc subpackage for rubygem based rpm
>       and I usually put Rakefile to -doc, not to main package )

The problem is, i can't even do "rake test" because loading of rakefile fails on dependencies.

> 
>   - But with your current rpm only ruby script with shebang have executable
>     permission (and not all *_test.rb have shebang) anyway, so judging by
>     my method should be possible.

In my current rpm, only two of 10 test files miss shebang line, in %install i add it to those two, and then set all 10 to +x with chmod.

>     ( By the way if scripts without shebang have executable permission, or
>       if scripts with shebang don't have exectable permission, rpmlint
>       warns about this ).

I know. That's why i do all this :)

> 
>     The reason I am talking about this is that I think hardcoding %test_files
>     list should be avoided unless impossible.  

It's not impossible, but i'd have to:
1. create a patch that adds shebang lines, so all _test.rb have it; and
2. call chmod +x, or specify executable bit with %attr

Do you think this would be OK ?

Well, then first of all how did you determine the list
of %test_files?

I asked one of the developers, he said test/*/*_test.rb

Well, for now I won't object to use %test_files. So
would you modify your spec file if there is something you want
to do so and reupload your spec/srpm again?

*** Bug 551565 has been marked as a duplicate of this bug. ***

OK, i did some changes:
- update to 2.2.16
- got rid of test_files macro in %check (replaced it with solution from comment #3)
- replaced test_files in %install with commands from Jeroen van Meeuwen's specfile for rubygem-haml (bug 551565)

http://mogurakun.web.runbox.net/rubygem-haml.spec
http://mogurakun.web.runbox.net/rubygem-haml-2.2.16-1.fc12.src.rpm

Well, I have not looked into the newest srpm yet, however
at least scratch build fails on rawhide:

http://koji.fedoraproject.org/koji/taskinfo?taskID=1903695

Note that rawhide uses rubygem-rack-1.1.0-1.fc13

(In reply to comment #12)
> Well, I have not looked into the newest srpm yet, however
> at least scratch build fails on rawhide:
> 
> http://koji.fedoraproject.org/koji/taskinfo?taskID=1903695
> 
> Note that rawhide uses rubygem-rack-1.1.0-1.fc13  

Actually rubygem-actionpack is broken with rack 1.1.0
(bug 552972)

(In reply to comment #13)
> Actually rubygem-actionpack is broken with rack 1.1.0
> (bug 552972)  

I applied a workaround on rawhide actionpack.
Now this package itself seems okay, then:

-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on my wiki page:
http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets
(Check "No one is reviewing")

Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------

(By the way the macro %ruby_sitelib is used nowhere so this macro
 definition can be removed)

ping?

ping again?

Hello,

sorry for late response. What deadline are we aiming for ?

Not a dealine issue, however usually if no response from the 
submitter is received within one month + one week, the bug 
is closed as NOTABUG so that other person can take over the 
submitted package.

http://fedoraproject.org/wiki/Policy_for_stalled_package_reviews

ping again?

*** Bug 562998 has been marked as a duplicate of this bug. ***

Hey, I can help out w/ reviewing / submitting this package but can we update it to the most recent version 2.2.19

http://gemcutter.org/gems/haml

I ask because apparently there are new haml dependencies which are not reflected in the current rpm spec, and are needed by haml for features needed in other software.

Specifically the yard and markuku (which in return depends on syntax) gem dependencies have been added 

https://bugzilla.redhat.com/show_bug.cgi?id=562997
https://bugzilla.redhat.com/show_bug.cgi?id=562993
https://bugzilla.redhat.com/show_bug.cgi?id=504470

Haml >= 2,2,19 is needed for the latest release of compass https://bugzilla.redhat.com/show_bug.cgi?id=563000

If you need any assistance shout our or check my spec (in the closed / duplicate bug) where I have all the deps and build setup.

Hi,

> I ask because apparently there are new haml dependencies which are not
> reflected in the current rpm spec, and are needed by haml for features needed
> in other software.

Yard and maruku are not new deps; I intentionally left them out, since those were build dependencies (that is, if you want to build the gem out of git tree), and i didn't want to introduce unnecessary deps.

I'll ask upstream, but afaik they still aren't a runtime (or rpm build time) dependency.

I know there's new haml; i'll be updating it shortly.

It seems the patch supplied to the sass engine test is not needed anymore.

I used: http://www.kanarip.com/custom/SPECS/rubygem-haml.spec / http://www.kanarip.com/custom/f12/SRPMS/rubygem-haml-2.2.19-1.fc12.src.rpm

Michal, ping?
(Just to be sure that I am also waiting for your another review
 request or your pre-review of other person's review request for
 sponsorship issue)

Mamoru, Michal, any progress with this gem ?

Hi,

2.2.20 packaged is here:

http://mogurakun.web.runbox.net/rubygem-haml-2.2.20-1.fc13.noarch.rpm
http://mogurakun.web.runbox.net/rubygem-haml-2.2.20-1.fc13.src.rpm
http://mogurakun.web.runbox.net/rubygem-haml.spec

Note 1: doesn't build on F12 (rails + rack 1.1.0)
Note 2: removed patch to sass engine test

Well, your srpm itself seems good now.
So as I wrote in my comment 14 and comment 25, now I am waiting
for your another review request submission (submitting another
gem-based rpm may be simper), or your pre-review of other person's
review request.

(In reply to comment #27)
> Note 1: doesn't build on F12 (rails + rack 1.1.0)

Being tracked on bug 552972. Would you (and also people watching this bug)
comment on the bug? If we can hear consent from more people to downgrade
rack on F-12 to 1.0.0, we can fix so more quickly.

It seems like Rack was recently downgraded to 1.0.0, which means this package should be build fine in Koji now (F-12).

If Michal would confirm this, I'm not seeing any issue which could prevent giving a fedora-review+ flag here. 

Also, this package is blocking couple of things (and is attractive for users as well). So instead of waiting for Michal's reviews I could take CVS import of this package if Michal agreed.

Sure, go ahead.

If Michal Fojtik is going to be the maintainer of this package,
I can approve this package (F-14/13/12 rebuild passed)

So would you want to be the maintainer of rubygem-haml,
Michal Fojtik?

I'm willing to help with maintenance as I need this dependency for another app.

So who is really going to be the maintainer for this package?
Michal Fojtik, would you agree with that Matthew will be the owner
for this package?

I'm sorry, I was gone for couple of days. 
I agree, Matthew could be maintainer.

Thank you for replying. Now I approve this.

------------------------------------------------------------
  This package (rubygem-haml) is APPROVED by mtasaka
------------------------------------------------------------

New Package CVS Request
=======================
Package Name: rubygem-haml
Short Description: XHTML/XML templating engine
Owners: mkent
Branches: F-11 F-12 F-13
InitialCC:

CVS done (by process-cvs-requests.py).

rubygem-haml-2.2.20-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/rubygem-haml-2.2.20-1.fc11

rubygem-haml-2.2.20-1.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/rubygem-haml-2.2.20-1.fc12

rubygem-haml-2.2.20-1.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/rubygem-haml-2.2.20-1.fc13

Closing.

rubygem-haml-2.2.20-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

rubygem-haml-2.2.20-1.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

rubygem-haml-2.2.20-1.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: rubygem-haml
New Branches: el5 el6
Owners: stahnma

Git done (by process-git-requests).

(In reply to comment #47)
> Package Change Request
> ======================
> Package Name: rubygem-haml
> New Branches: el5 el6
> Owners: stahnma

Can you please import this gem into EPEL6?

Error: No Package found for rubygem(haml)

Thanks!