Bug 1132671 - Review Request: rubygem-compass-import-once - Ruby Module to speed up Sass compilation
Summary: Review Request: rubygem-compass-import-once - Ruby Module to speed up Sass co...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Vít Ondruch
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1152676
TreeView+ depends on / blocked
 
Reported: 2014-08-21 19:31 UTC by Mo Morsi
Modified: 2015-02-15 03:05 UTC (History)
2 users (show)

Fixed In Version: rubygem-compass-import-once-1.0.5-4.fc20
Clone Of:
Environment:
Last Closed: 2015-02-15 02:57:13 UTC
Type: ---
Embargoed:
vondruch: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Mo Morsi 2014-08-21 19:31:44 UTC
Spec URL: https://mmorsi.fedorapeople.org/staging/rubygem-compass-import-once.spec
SRPM URL: http://mmorsi.fedorapeople.org/staging/rubygem-compass-import-once-1.0.1-1.fc20.src.rpm
Description:
Compass module which changes the Sass's @import directive to only import a file once.

Fedora Account System Username: mmorsi
Additional Info: Required to update rubygem-compass to the latest version

Comment 1 Vít Ondruch 2014-08-28 07:51:36 UTC
I'll take this for a review.

Comment 2 Vít Ondruch 2014-08-28 07:55:18 UTC
I hope the correct SRPM url is https://mmorsi.fedorapeople.org/staging/rubygem-compass-import-once-1.0.5-1.fc20.src.rpm , since the one you provided does not work ...

Comment 3 Vít Ondruch 2014-08-28 08:07:46 UTC
* The .spec differences
  - The .spec file you posted differs from the one included in SRPM. It is just
    small difference in you name in %changelog, so no big deal.

* Update to the latest guidelines
  - The .spec file looks to be generated by old gem2rpm and does not conform
    to the latest Ruby packaging guidelines. Please remove the Requires, which
    are now autogenerated and Provides.

* Test suite is not executed.
  - Please execute the test suite.
  - Although it is not shipped with the gem, it is available upstream [1].

* Move VERSION file into the main package
  - It seems to be runtime dependency. It is read by
    lib/compass/import-once/version.rb


[1] https://github.com/Compass/compass/tree/stable/import-once/test

Comment 4 Mo Morsi 2014-08-28 17:40:11 UTC
Yes regarding the URL, copy-n-paste err. Updated:

Spec: https://mmorsi.fedorapeople.org/staging/rubygem-compass-import-once.spec
SRPM: https://mmorsi.fedorapeople.org/staging/rubygem-compass-import-once-1.0.5-2.fc20.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=7476134

Incorporated feedback. Same note about test suite / minitest5 compatibility as w/ compass-core

Comment 5 Mo Morsi 2014-09-18 21:47:01 UTC
Updated

Spec: https://mmorsi.fedorapeople.org/staging/rubygem-compass-import-once.spec
SRPM: https://mmorsi.fedorapeople.org/staging/rubygem-compass-import-once-1.0.5-3.fc20.src.rpm

Unfortunately there is only one test module in this one and it depends on sass-globbing which currently is awaiting a license clarification:

https://github.com/chriseppstein/sass-globbing/issues/24

Shouldn't be a blocker, verified test ran when sass-globbing was installed manually locally, commented test out, will uncomment when that makes it in.

Comment 6 Vít Ondruch 2014-12-22 13:47:57 UTC
* The spec file differs from SRPM
  - There is BR: rubygem(sass-globbing) enabled in SRPM while disabled in the
    .spec you provided. The latter seems to be what you probably wanted.

* Test suite
  - It seems that 

    sed -i '/sass-globbing/ s/^/#/' test/test_helper.rb
    mv test/fixtures/with_globbing.scss{,.disable}

    Should be enough to execute the test suite without sass-globbing.

* Patching using %patch0 macro
  - I'd suggest to apply the patch using "%patch0 -p1" in %prep section instead
    of the %build section.

Otherwise, the package looks good => APPROVED

Comment 8 Mo Morsi 2015-01-05 19:36:58 UTC
New Package SCM Request
=======================
Package Name: rubygem-compass-import-once
Short Description:  The Compass Stylesheet Library Import Once Plugin
Upstream URL: http://rubygems.org/gems/compass-import-once
Owners: mmorsi
Branches: f20 f21
InitialCC:

Comment 9 Gwyn Ciesla 2015-01-05 21:11:20 UTC
Git done (by process-git-requests).

Comment 10 Fedora Update System 2015-01-27 00:13:55 UTC
rubygem-compass-import-once-1.0.5-4.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/rubygem-compass-import-once-1.0.5-4.fc21

Comment 11 Fedora Update System 2015-01-27 00:14:41 UTC
rubygem-compass-import-once-1.0.5-4.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/rubygem-compass-import-once-1.0.5-4.fc20

Comment 12 Fedora Update System 2015-01-28 19:56:44 UTC
rubygem-compass-import-once-1.0.5-4.fc20 has been pushed to the Fedora 20 testing repository.

Comment 13 Fedora Update System 2015-02-15 02:57:13 UTC
rubygem-compass-import-once-1.0.5-4.fc21 has been pushed to the Fedora 21 stable repository.

Comment 14 Fedora Update System 2015-02-15 03:05:57 UTC
rubygem-compass-import-once-1.0.5-4.fc20 has been pushed to the Fedora 20 stable repository.


Note You need to log in before you can comment on or make changes to this bug.