Bug 207473
Summary: | Review Request: ruby-activerecord - Implements the ActiveRecord pattern for ORM | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | David Lutterkort <lutter> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | hbrock, k.georgiou |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-12-14 23:48:23 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | 207472 | ||
Bug Blocks: | 163779 |
Description
David Lutterkort
2006-09-21 11:01:39 UTC
rpmlint says: E: ruby-activerecord script-without-shellbang /usr/share/doc/ruby-activerecord-1.14.2/README because it gets confused by the first line of the README. Should b ignorable. (In reply to comment #1) > rpmlint says: > E: ruby-activerecord script-without-shellbang > /usr/share/doc/ruby-activerecord-1.14.2/README > > because it gets confused by the first line of the README. No it doesen't, README is installed executable... Is active_support really required? I don't see anything in the code that uses it. (In reply to comment #2) > (In reply to comment #1) > > rpmlint says: > > E: ruby-activerecord script-without-shellbang > > /usr/share/doc/ruby-activerecord-1.14.2/README > > > > because it gets confused by the first line of the README. > > No it doesen't, README is installed executable... Ouch .. how embarrassing. Fixed now Spec URL: http://people.redhat.com/dlutter/yum/spec/ruby-activerecord.spec SRPM URL: http://people.redhat.com/dlutter/yum/SRPMS/ruby-activerecord-1.14.2-2.src.rpm (In reply to comment #3) > Is active_support really required? I don't see anything in the code that uses it. Yes, it is .. $sitelibdir/active_record.rb requires it and gets grumpy if it's not there. Since active_support modifies some of the core classes, users of active_record can (and should) expect those additional methods to be there, no matter how active_record was installed (tarball, gem, or rpm) Updated to latest upstream: Spec URL: http://people.redhat.com/dlutter/yum/spec/ruby-activerecord.spec SRPM URL: http://people.redhat.com/dlutter/yum/SRPMS/ruby-activerecord-1.14.4-1.src.rpm I noted the presence of a nice test suite, and realized that it should actually be possible to run at least some of it because sqlite doesn't need a configured database server. So I set out to get the test suite to run with: %check cd test ruby -I "connections/native_sqlite3" base_test.rb In order to get this to work, it's necessary to add some BRs: ruby(active_support), ruby(sqlite3), ruby-irb The first two are pretty obvious, but the third was not. It looks like the ruby-activesupport package needs a runtime dependency on ruby-irb. In any case, the test suite unfortunately fails: Using native SQLite3 SQLite3 database not found at /builddir/build/BUILD/activerecord-1.14.4/test/fixtures/fixture_database.sqlite3. Rebuilding it. Executing 'sqlite3 /builddir/build/BUILD/activerecord-1.14.4/test/fixtures/fixture_database.sqlite3 "create table a (a integer); drop table a;"' SQLite3 database not found at /builddir/build/BUILD/activerecord-1.14.4/test/fixtures/fixture_database_2.sqlite3. Rebuilding it. Executing 'sqlite3 /builddir/build/BUILD/activerecord-1.14.4/test/fixtures/fixture_database_2.sqlite3 "create table a (a integer); drop table a;"' -- create_table(:taggings, {:force=>true}) -> 0.0158s -- create_table(:tags, {:force=>true}) -> 0.0160s -- create_table(:categorizations, {:force=>true}) -> 0.0158s -- add_column(:posts, :taggings_count, :integer, {:default=>0}) -> 0.0182s -- add_column(:authors, :author_address_id, :integer) -> 0.0162s -- create_table(:author_addresses, {:force=>true}) -> 0.0173s -- create_table(:author_favorites, {:force=>true}) -> 0.0240s Loaded suite base_test Started .......................................................................................................F............. Finished in 0.519048 seconds. 1) Failure: test_to_xml_including_multiple_associations(BasicsTest) [base_test.rb:1248]: <false> is not true. 117 tests, 297 assertions, 1 failures, 0 errors It would be good to figure out what's going wrong here just in case there is an actual problem with the code on this platform. Review: * source files match upstream: ce66299a7fe99fdaf2c9747e850560b6 activerecord-1.14.4.tgz * package meets naming and packaging guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * dist tag is present. * build root is correct. * license field matches the actual license. * license is open source-compatible. License text not included upstream. * latest version is being packaged. * BuildRequires are proper. * %clean is present. * package builds in mock (development, x86_64). * package installs properly * rpmlint is silent. * final provides and requires are sane: ruby(active_record) = 1.14.4 ruby-activerecord = 1.14.4-1.fc7 = ruby(abi) = 1.8 ruby(active_support) ? Not sure about %check. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no scriptlets present. * code, not content. * documentation is small, so no -docs subpackage is necessary. * %docs are not necessary for the proper functioning of the package. Ah, it turns out this is a bug in the test: http://dev.rubyonrails.org/ticket/5268 I suggest just applying that patch and enabling the tests, unless you have some compelling reason why we shouldn't try to run them. I enabled the tests (and added the necessary BR's) I had to make the test running conditional so that the specfile works for RHEL4 too (RHEL4 doesn't have sqlite3) Spec URL: http://people.redhat.com/dlutter/yum/spec/ruby-activerecord.spec SRPM URL: http://people.redhat.com/dlutter/yum/SRPMS/ruby-activerecord-1.14.4-2.src.rpm I enabled the tests (and added the necessary BR's) I had to make the test running conditional so that the specfile works for RHEL4 too (RHEL4 doesn't have sqlite3) Spec URL: http://people.redhat.com/dlutter/yum/spec/ruby-activerecord.spec SRPM URL: http://people.redhat.com/dlutter/yum/SRPMS/ruby-activerecord-1.14.4-2.src.rpm My apologies; somehow this slipped past me. Still builds fine on rawhide, tests are run and all pass: Finished in 0.657058 seconds. 117 tests, 298 assertions, 0 failures, 0 errors 1.14.4 is still current. Everything else looks good. Already did the review, and everything these still applies except that %check has been added. APPROVED Imported and successfully built as plague job 23782 |