https://bugzilla.redhat.com/show_bug.cgi?id=1859414 --- Comment #7 from Pavel Valena <pvalena@xxxxxxxxxx> --- (In reply to Vít Ondruch from comment #3) > The URL ^^ does not work. I think I am supposed to use the following URLs > now, right? > > https://copr-be.cloud.fedoraproject.org/results/pvalena/ruby-on-rails/fedora- > rawhide-x86_64/01591362-rubygem-actionmailbox/rubygem-actionmailbox.spec > https://copr-be.cloud.fedoraproject.org/results/pvalena/ruby-on-rails/fedora- > rawhide-x86_64/01591362-rubygem-actionmailbox/rubygem-actionmailbox-6.0.3.1- > 1.fc33.src.rpm(In reply to Pavel Yes, sorry I forgot it'll get cleaned up on rebuild. 1591362 is a correct (latest) build. FTR, this is git-based link to spec file: https://copr-dist-git.fedorainfracloud.org/cgit/pvalena/ruby-on-rails/rubygem-actionmailbox.git/plain/rubygem-actionmailbox.spec?id=9e33f0b7121acd9236d874f1a50adebd40eb41c8 > * Never release available upstream > - actionmailbox 6.0.3.2 is available upstream already. But I guess you want to keep this in sync with the rest of the RoR, so no big deal ATM. Yes, I have most already built in side-tag: https://koji.fedoraproject.org/koji/builds?userID=&tagID=26290&order=-build_id&inherited=0&latest=1 I will run it through "update" automation (build in side-tag once more), when 6.0.3.1 is merged in master. > * Summary and description > - The summary and description differs from the upstream versions, is that intentional? I will fix that. Most probably it was changed when bumping to 6.0.3 (I didn't check those differences much). > * tests and tools in separate archives > - What is the reason? Wouldn't it be enough to keep them in one archive? rails-*-tools.txz is one archive with tools shared accross most of RoR, as it's needed for running most of the test suites. I've decided to archive/manage it separately. > - Also, while minor nit, there is probably missing '/' in the command for tests. Doesn't have to be there, and AFAIK it doesn't change the behaviour. I'll add it for readability. > * Missing space after `Requires:` > - There is missing space in `-doc` subpackage: Fixed. > * Keep the default generated BRs together: > - It would be nice to keep these together: > - Mixing rubygems-devel in between other requires for instance does not make the review/maintenance easier. Right, the intent was to have them sorted alphabetically, as the comment says (I've used `sort -u`). But sure, I will revert that. I have no preference (as the whole `BuildRequires:` concept is wrong IMO). > * Circular dependency with rubygem-rails > - I think that using `rails` gem on various places is shortcut which brings its own issues. I think it would be better to list all the dependencies excluding `rails` itself. Sure, let's trim those down. (Note that it's only a runtime dependency, so it does't matter until something depends on actionmailbox at buildtime.) > * Keep the original tests in place > - I prefer to keep the expanded tests in place and copy them to the location they needs to be. But it might be confusing either way. Ok. Is there any benefit to having them duplicated? > * Simpler test execution > - There is too much boilerplate in the `%check` section. I think this would be enough: Sure, let's do that instead. I somehow preferred the current (generic) approach, but it makes sense to trim it down. Thanks for the review! _ _ _ _ The changed spec will will follow in a separate comment. -- 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@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
