https://bugzilla.redhat.com/show_bug.cgi?id=1859414
--- Comment #7 from Pavel Valena <[email protected]> --- (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 -- [email protected] To unsubscribe send an email to [email protected] 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/[email protected]
