[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233 --- Comment #13 from Fedora Update System upda...@fedoraproject.org 2012-01-15 15:10:32 EST --- rubygem-rack-protection-1.2.0-2.fc16 has been pushed to the Fedora 16 stable repository. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|ON_QA |CLOSED Fixed In Version||rubygem-rack-protection-1.2 ||.0-2.fc16 Resolution||ERRATA Last Closed||2012-01-13 23:02:04 --- Comment #12 from Fedora Update System upda...@fedoraproject.org 2012-01-13 23:02:04 EST --- rubygem-rack-protection-1.2.0-2.fc16 has been pushed to the Fedora 16 stable repository. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|MODIFIED|ON_QA --- Comment #11 from Fedora Update System upda...@fedoraproject.org 2012-01-05 16:06:49 EST --- rubygem-rack-protection-1.2.0-2.fc16 has been pushed to the Fedora 16 testing repository. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233
--- Comment #7 from Michal Fojtik mfoj...@redhat.com 2012-01-04 06:24:51 EST
---
(In reply to comment #6)
- Move %{geminstdir}/README.md to the -doc subpackage, if it is not
needed for
runtime (which I believe it isn't) and mark it with %doc.
- Also, mark %{gemdir}/doc/%{gemname}-%{version} with %doc.
Actually I don't think marking -doc subpackage files with %doc is necessary.
Could you point me to a guideline where this is required?
I believe that there is no specific guideline for this. But if you take a look
at it from the logical point of view, you have two types of files in your
-doc
subpackage:
- The Rakefile and spec/ directory, which are not needed for runtime, so you
moved them to -doc subpackage (which is completely ok, I think), but they are
not documentation.
- The real documentation (README.md and the doc directory).
So, to me, it makes sense to distinct these two.
Good point. My general through was that once the subpackage is marked as '-doc'
the files are already recognized as %doc and thus don't need additional
marking.
Anyway, I'll mark them, not a blocker for me of course ;-)
Additional comments:
- I think it is clearer not to remove the files by rm, but use %exclude in
%files. But this is just my opinion, so not a blocker.
- As for the macros vs. commands thing: There are also macros for commands
like
rm, so it may be good to use them, once you decide to use macros for some
commands. But at this stage, it doesn't have the feeling of inconsistency, so
not a blocker. (BTW, I think that using macros for things like mkdir -p is
not necessary, but again, just my opinion.)
- The link in comment #5 points to the first release srpm, so when importing
to
fedpkg, please make sure to import the second one :)
Sure, I'll try to fix that before importing to Fedora (EPEL). Thanks for review
Bohuslav!
--
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233 Michal Fojtik mfoj...@redhat.com changed: What|Removed |Added Flag||fedora-cvs? --- Comment #8 from Michal Fojtik mfoj...@redhat.com 2012-01-04 06:26:25 EST --- New Package SCM Request === Package Name: rubygem-rack-protection Short Description: Ruby gem that protects against typical web attacks Owners: mfojtik Branches: f16 el6 -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233 --- Comment #9 from Jon Ciesla limburg...@gmail.com 2012-01-04 08:42:35 EST --- Git done (by process-git-requests). -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|ASSIGNED|MODIFIED -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233 --- Comment #10 from Fedora Update System upda...@fedoraproject.org 2012-01-04 09:26:05 EST --- rubygem-rack-protection-1.2.0-2.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/rubygem-rack-protection-1.2.0-2.fc16 -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233 Michal Fojtik mfoj...@redhat.com changed: What|Removed |Added Blocks||691731 -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233 Bohuslav Kabrda bkab...@redhat.com changed: What|Removed |Added Status|NEW |ASSIGNED CC||bkab...@redhat.com AssignedTo|nob...@fedoraproject.org|bkab...@redhat.com Flag||fedora-review? --- Comment #2 from Bohuslav Kabrda bkab...@redhat.com 2012-01-03 07:15:45 EST --- I'm taking this one. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233
--- Comment #3 from Bohuslav Kabrda bkab...@redhat.com 2012-01-03 07:56:05
EST ---
- The package FTBTS [1]. Please do a Koji build before every review to see if
it passes.
-- You need to BR: rubygem(rspec-core) instead of rubygem(rspec).
-- You also need to BR: rubygem(rack-test) for running the specs.
- Could you specify what is the advantage of running the specs with ruby -S
rspec spec, when rspec spec suffices? This is not a blocker, but why not
make things as simple as possible?
- Do you intend to place the package into EPEL, too? If not, please remove the
unnecessary BuildRoot tag (see [2] for more info).
- Please be consistent in usage of macros for shell commands and the commands
themselves. For example, you use both %{__mkdir_p} macro and mkdir -p
command. So decide whether you want to use macros or commands and don't mix the
two.
- Consider excluding the cached gem, as it is not an unnecessary payload, not
needed for RPM package.
- Mark %{geminstdir}/License with %doc.
- Move %{geminstdir}/README.md to the -doc subpackage, if it is not needed for
runtime (which I believe it isn't) and mark it with %doc.
- Also, mark %{gemdir}/doc/%{gemname}-%{version} with %doc.
[1] http://koji.fedoraproject.org/koji/taskinfo?taskID=3615389
[2] https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
--
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233
--- Comment #4 from Michal Fojtik mfoj...@redhat.com 2012-01-03 11:09:52 EST
---
(In reply to comment #3)
Hi Bohuslav
First thanks for reviewing this package!
- The package FTBTS [1]. Please do a Koji build before every review to see if
it passes.
-- You need to BR: rubygem(rspec-core) instead of rubygem(rspec).
-- You also need to BR: rubygem(rack-test) for running the specs.
All done.
- Could you specify what is the advantage of running the specs with ruby -S
rspec spec, when rspec spec suffices? This is not a blocker, but why not
make things as simple as possible?
We used it in thin package. However this form sounds better and shorter for me.
Fixed in -2.
- Do you intend to place the package into EPEL, too? If not, please remove the
unnecessary BuildRoot tag (see [2] for more info).
This package will be imported to EPEL. I'll exclude BuildRoot from Fedora
packages
before import.
- Please be consistent in usage of macros for shell commands and the commands
themselves. For example, you use both %{__mkdir_p} macro and mkdir -p
command. So decide whether you want to use macros or commands and don't mix
the
two.
Done.
- Consider excluding the cached gem, as it is not an unnecessary payload, not
needed for RPM package.
Done.
- Mark %{geminstdir}/License with %doc.
Done,
- Move %{geminstdir}/README.md to the -doc subpackage, if it is not needed for
runtime (which I believe it isn't) and mark it with %doc.
- Also, mark %{gemdir}/doc/%{gemname}-%{version} with %doc.
Actually I don't think marking -doc subpackage files with %doc is necessary.
Could you point me to a guideline where this is required?
=
koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3615827
rpmlint:
rubygem-rack-protection.noarch: I: enchant-dictionary-not-found en_US
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
* Mon Jan 03 2012 Michal Fojtik mfoj...@redhat.com - 1.2.0-2
- Fixed BR
- Marked documentation file with doc tag
- Changed the way how to run rspec tests
--
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233 --- Comment #5 from Michal Fojtik mfoj...@redhat.com 2012-01-03 11:11:11 EST --- Revision -2: Spec: http://omicron.mifo.sk/rubygem-rack-protection.spec SRPM: http://omicron.mifo.sk/rubygem-rack-protection-1.2.0-1.fc14.src.rpm -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233
Bohuslav Kabrda bkab...@redhat.com changed:
What|Removed |Added
Flag|fedora-review? |fedora-review+
--- Comment #6 from Bohuslav Kabrda bkab...@redhat.com 2012-01-04 01:57:32
EST ---
- Move %{geminstdir}/README.md to the -doc subpackage, if it is not needed
for
runtime (which I believe it isn't) and mark it with %doc.
- Also, mark %{gemdir}/doc/%{gemname}-%{version} with %doc.
Actually I don't think marking -doc subpackage files with %doc is necessary.
Could you point me to a guideline where this is required?
I believe that there is no specific guideline for this. But if you take a look
at it from the logical point of view, you have two types of files in your -doc
subpackage:
- The Rakefile and spec/ directory, which are not needed for runtime, so you
moved them to -doc subpackage (which is completely ok, I think), but they are
not documentation.
- The real documentation (README.md and the doc directory).
So, to me, it makes sense to distinct these two.
Additional comments:
- I think it is clearer not to remove the files by rm, but use %exclude in
%files. But this is just my opinion, so not a blocker.
- As for the macros vs. commands thing: There are also macros for commands like
rm, so it may be good to use them, once you decide to use macros for some
commands. But at this stage, it doesn't have the feeling of inconsistency, so
not a blocker. (BTW, I think that using macros for things like mkdir -p is
not necessary, but again, just my opinion.)
- The link in comment #5 points to the first release srpm, so when importing to
fedpkg, please make sure to import the second one :)
Since none of my additional comments are blockers, this package is APPROVED.
--
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 771233] Review Request: rubygem-rack-protection - Ruby gem that protects against typical web attacks
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=771233 --- Comment #1 from Michal Fojtik mfoj...@redhat.com 2012-01-02 13:44:23 EST --- This package is need for updating Sinatra to 1.3.2. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
