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

List overview All Threads
Download

newer

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

older

[Bug 967332] New: Review Request:...

[Bug 967333] New: Review Request:...

Red Hat Bugzilla

12 Jul 2012 12 Jul '12

1:40 p.m.

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

Bug ID: 839649 QA Contact: extras-qa@fedoraproject.org Severity: medium Version: rawhide Priority: medium CC: notting@redhat.com, package-review@lists.fedoraproject.org Assignee: nobody@fedoraproject.org Summary: Review Request: rubygem-rails_best_practices - a code metric tool for rails codes, written in Ruby. Regression: --- Story Points: --- Classification: Fedora OS: Linux Reporter: mzatko@redhat.com Type: --- Documentation: --- Hardware: All Mount Type: --- Status: NEW Component: Package Review Product: Fedora

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

Show replies by date

Red Hat Bugzilla

12 Jul 12 Jul

2:20 p.m.

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

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

Michal Fojtik mfojtik@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |mfojtik@redhat.com

--- Comment #1 from Michal Fojtik mfojtik@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.

Red Hat Bugzilla

3 Sep 3 Sep

3:36 p.m.

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

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

--- Comment #2 from Maros Zatko mzatko@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.

Red Hat Bugzilla

4 Sep 4 Sep

8:58 a.m.

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

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

--- Comment #3 from Maros Zatko mzatko@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.

Red Hat Bugzilla

9 a.m.

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

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

Michal Fojtik mfojtik@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review+

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

-- You are receiving this mail because: You are on the CC list for the bug.

Red Hat Bugzilla

2 Oct 2 Oct

8:26 p.m.

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

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

Mo Morsi mmorsi@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |mmorsi@redhat.com

--- Comment #5 from Mo Morsi mmorsi@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.

Red Hat Bugzilla

4 Oct 4 Oct

1:11 p.m.

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

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

--- Comment #6 from Maros Zatko mzatko@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.

Red Hat Bugzilla

18 Oct 18 Oct

11:18 a.m.

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

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

Josef Stribny jstribny@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |jstribny@redhat.com

--- Comment #7 from Josef Stribny jstribny@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.

Red Hat Bugzilla

10 Dec 10 Dec

4:40 p.m.

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

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

Josef Stribny jstribny@redhat.com changed:

-- 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=ww936yUSlH&a=cc_unsubscribe

Red Hat Bugzilla

4:54 p.m.

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

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

--- Comment #8 from Josef Stribny jstribny@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=p73aQlq63O&a=cc_unsubscribe

Red Hat Bugzilla

11 Dec 11 Dec

8:16 a.m.

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

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

Vít Ondruch vondruch@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |vondruch@redhat.com

--- Comment #9 from Vít Ondruch vondruch@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=quBUKsQrZK&a=cc_unsubscribe

Red Hat Bugzilla

8:27 a.m.

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

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

--- Comment #10 from Vít Ondruch vondruch@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=lRWw623IXg&a=cc_unsubscribe

Red Hat Bugzilla

2:43 p.m.

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

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

Josef Stribny jstribny@redhat.com changed:

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

-- 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=VWje39jbDy&a=cc_unsubscribe

Red Hat Bugzilla

22 Apr 22 Apr

10:33 a.m.

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

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

--- Comment #11 from Josef Stribny jstribny@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=B7oJmY8ZLj&a=cc_unsubscribe

Red Hat Bugzilla

4 Nov 4 Nov

10:50 a.m.

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

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

Josef Stribny jstribny@redhat.com changed:

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

Red Hat Bugzilla

4 Jan 4 Jan

5:56 a.m.

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

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

John Skeoch jskeoch@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Assignee|jstribny@redhat.com |hhorak@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

Red Hat Bugzilla

8:57 a.m.

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

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

Vít Ondruch vondruch@redhat.com changed:

--- Comment #13 from Vít Ondruch vondruch@redhat.com --- 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

Red Hat Bugzilla

8:57 a.m.

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

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

Vít Ondruch vondruch@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Assignee|hhorak@redhat.com |vondruch@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

3098

Age (days ago)

4369

Last active (days ago)

package-review@lists.fedoraproject.org

17 comments

1 participants

tags (0)

participants (1)

Red Hat Bugzilla