[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

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

Vít Ondruch  changed:

   What|Removed |Added

   Assignee|hho...@redhat.com   |vondr...@redhat.com



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

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

Vít Ondruch  changed:

   What|Removed |Added

 Status|ASSIGNED|CLOSED
 Blocks||201449 (FE-DEADREVIEW)
 Resolution|--- |NOTABUG
  Flags|fedora-review?  |
   |needinfo?(mza...@redhat.com |
   |)   |
Last Closed||2016-01-04 03:57:33



--- Comment #13 from Vít Ondruch  ---
Closing this stalled review.


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=201449
[Bug 201449] FE-DEADREVIEW -- Reviews stalled due to lack of submitter
response should be blocking this bug.
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

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

John Skeoch  changed:

   What|Removed |Added

   Assignee|jstri...@redhat.com |hho...@redhat.com



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

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

Josef Stribny jstri...@redhat.com changed:

   What|Removed |Added

 CC||mza...@redhat.com
  Flags||needinfo?(mza...@redhat.com
   ||)



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=839649

--- Comment #11 from Josef Stribny jstri...@redhat.com ---
Maros, are you still working on this? If so, please notice the change in the
guidelines regarding Fedora 19 [1] and make necessary changes to the spec file.

1, Please change:
- Requires: ruby(abi) = %{rubyabi} to Requires: ruby(release)
- BuildRequires: ruby(abi) = %{rubyabi} to BuildRequires: ruby(release)

2, Use %gem_install macro instead of calling gem install directly

[1] https://fedoraproject.org/wiki/Packaging:Ruby

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug 
https://bugzilla.redhat.com/token.cgi?t=B7oJmY8ZLja=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=839649

Vít Ondruch vondr...@redhat.com changed:

   What|Removed |Added

 CC||vondr...@redhat.com

--- Comment #9 from Vít Ondruch vondr...@redhat.com ---
(In reply to comment #8)
 5, Why do you list subfolders from
 %{gem_instdir}/spec/rails_best_practices/? Isn't enough to list it as one
 folder only?

I would put it differently. There is nothing wrong in listing subfolders. This
may help to assure, that the specific folders are included in package. However,
as it is done currently, only the files are owned by package, not the
directories itself. And that is wrong. This applies not just to
%{gem_instdir}/spec/ but to %{gem_instdir}/assets/ as well.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug 
https://bugzilla.redhat.com/token.cgi?t=quBUKsQrZKa=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=839649

--- Comment #10 from Vít Ondruch vondr...@redhat.com ---
(In reply to comment #8)
 2, Get rid of 
 
 Requires: ruby

Agree with this.

 BuildRequires: ruby

Don't fully agree. It is safer option to include BR: ruby ATM.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug 
https://bugzilla.redhat.com/token.cgi?t=lRWw623IXga=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=839649

Josef Stribny jstri...@redhat.com changed:

   What|Removed |Added

   Assignee|nob...@fedoraproject.org|jstri...@redhat.com
  Flags|fedora-review+  |
  Flags||fedora-review?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug 
https://bugzilla.redhat.com/token.cgi?t=ww936yUSlHa=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=839649

--- Comment #8 from Josef Stribny jstri...@redhat.com ---
Because it is still unresolved, I am taking this to continue with the review.

1, Summary should start with a capital A as mentioned above

2, Get rid of 

Requires: ruby
BuildRequires: ruby

Requires: ruby(api) will require this for you and you may use it with other
interpretations of Ruby.

3, Mark %{gem_instdir}/README.md and %{gem_instdir}/MIT_LICENSE as %doc

4, Move %{gem_instdir}/rails_best_practices.gemspec and %{gem_instdir}/Gemfile
to doc subpackage

5, Why do you list subfolders from %{gem_instdir}/spec/rails_best_practices/?
Isn't enough to list it as one folder only?

6, Consider upgrading it to the latest upstream version (1.13.1)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug 
https://bugzilla.redhat.com/token.cgi?t=p73aQlq63Oa=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

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

Josef Stribny jstri...@redhat.com changed:

   What|Removed |Added

 CC||jstri...@redhat.com

--- Comment #7 from Josef Stribny jstri...@redhat.com ---
Hi,

I believe Summary should start with a capital A for consistency as in How to
create an RPM package guide [1] and as Michal Fojtik already suggested.

(This is an informal review.)

[1] https://fedoraproject.org/wiki/How_to_create_an_RPM_package

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

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

--- Comment #6 from Maros Zatko mza...@redhat.com ---
updated:
Spec URL: http://v3.sk/~hexo/rpm/rubygem-rails_best_practices.spec
SRPM URL:
http://v3.sk/~hexo/rpm/rubygem-rails_best_practices-1.10.1-4.fc17.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

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

Mo Morsi mmo...@redhat.com changed:

   What|Removed |Added

 CC||mmo...@redhat.com

--- Comment #5 from Mo Morsi mmo...@redhat.com ---
Couple post review-nits

* the spec file should be named rubygem-rails_best_practices

* can you run the spec suite in a check section in the specfile?

* please move the spec suite into the docs subpackage

* please rm the files you exclude earlier in the spec and remove those excludes
from the files section

* slim is listed as a dev dependency on rubygems.org but is not referenced in
this spec, is it needed?

Thanks.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

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

--- Comment #3 from Maros Zatko mza...@redhat.com ---
Once again, with compacted file list

Spec URL: http://v3.sk/~hexo/rpm/rails_best_practices.spec
SRPM URL:
http://v3.sk/~hexo/rpm/rubygem-rails_best_practices-1.10.1-3.fc17.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

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

Michal Fojtik mfoj...@redhat.com changed:

   What|Removed |Added

  Flags||fedora-review+

--- Comment #4 from Michal Fojtik mfoj...@redhat.com ---
Thanks, REVIEW+

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

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

--- Comment #2 from Maros Zatko mza...@redhat.com ---
Thanks for your review, I've updated spec, so

Spec URL: http://v3.sk/~hexo/rpm/rails_best_practices.spec
SRPM URL:
http://v3.sk/~hexo/rpm/rubygem-rails_best_practices-1.10.1-2.fc17.src.rpm
Description: a code metric tool for rails codes, written in Ruby.
Fedora Account System Username: mzatko

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 839649] Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby.

[bugzilla]

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

Michal Fojtik mfoj...@redhat.com changed:

   What|Removed |Added

 CC||mfoj...@redhat.com

--- Comment #1 from Michal Fojtik mfoj...@redhat.com ---
Review:

- Summary  %description should start with the capital 'A'
- No tests

Please check if those tests are not included in gem, if so, I would ask
upstream to add them. But not a review-blocker:

https://github.com/railsbp/rails_best_practices/tree/master/spec

- The list of files in the %files section looks a bit ugly, it is possible to
somehow compress it? Like using directories rather than listing all files?

- Please exclude the '.yardoc/*' files, then are not needed for the gem

- Also consider removing this files:

%{gem_instdir}/.gemtest
%{gem_instdir}/.gitignore
%{gem_instdir}/.rspec
%{gem_instdir}/.rvmrc
%{gem_instdir}/.travis.yml
%{gem_instdir}/Gemfile
%{gem_instdir}/Gemfile.lock
%{gem_instdir}/Guardfile

- The MIT_LICENSE should go into main %files section
- I also think the 'License: GPLv2+ or Ruby' is wrong, since the license seems
to be MIT.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review