[Bug 1992914] Review Request: rubygem-middleware - Generalized implementation of the middleware abstraction for Ruby

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--- Comment #2 from Pavel Valena <pvalena@xxxxxxxxxx> ---
(In reply to Mamoru TASAKA from comment #1)
> Taking. Would you instead review my review request?
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=1992931

Thanks!

I did submit this, at it was blocking a colleague, and was long overdue. I'll
not be able to do the review in the near time, but I'll ask the colleague to
take it instead.

> 
> Some comments:
> 
> * release number and last %changelog entry mismatch. (%changelog in spec
> file in .src.rpm ends with -10)

Sorry, I've fixed this in the attached .spec file, but forgot to do so in the
SRPM [1].

>   (New %changelog date format (including time and time difference) does not
> seem to be supported on
>    rpmlint, also this would cause error / warning when building this when
> doing 'fedpkg build' after
>    importing this to fedora dist-git. I suggest to use old date format.)

I'm not getting any rpmlint error [2]. But I'm fine with the old format.

> 
> * Check if files in %doc are really needed.
>   Especially, spec/ Rakefile Gemfile should not be needed. I also suggest to
> remove %{gem_instdir}/%{gem_name}.gemspec .
> 

This seems like a divergence from upstream to me. I'd prefer leave it in a
subpackage for inspection purposes, as it came with the .gem. There's no harm
done to have it, I think.

> 
> * Note (not a blocker):
>   rspec -I rspec spec now causes warnings. Please consider to check
>  
> https://github.com/mitchellh/middleware/pull/15/commits/
> f559ee1a5fbc625b3a629f64d03f4ce249ea2fa0

That's the only PR I've not checked. Thanks! I've applied the patch, and
updated the SRPM+Spec.

https://koji.fedoraproject.org/koji/taskinfo?taskID=73716696

[1]
https://github.com/fedora-distgit/rubygem-middleware/blob/rawhide/rubygem-middleware.spec#L70
[2] https://git.io/JRDca


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux