[Bug 1132670] Review Request: rubygem-compass-core - The Compass core stylesheet library

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1132670
Bug 1132670 depends on bug 1142508, which changed state.

Bug 1142508 Summary: Review Request: rubygem-true - Testing framework for Sass 
libraries
https://bugzilla.redhat.com/show_bug.cgi?id=1142508

   What|Removed |Added

 Status|ASSIGNED|CLOSED
 Resolution|--- |NOTABUG




-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org

[Bug 1132670] Review Request: rubygem-compass-core - The Compass core stylesheet library

[bugzilla]

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

Vít Ondruch  changed:

   What|Removed |Added

 Status|ASSIGNED|CLOSED
   Fixed In Version||rubygem-compass-core-1.0.1-
   ||4.fc22
 Resolution|--- |CURRENTRELEASE
Last Closed||2015-09-01 04:50:21



-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1132670] Review Request: rubygem-compass-core - The Compass core stylesheet library

[bugzilla]

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



--- Comment #9 from Jon Ciesla limburg...@gmail.com ---
Git done (by process-git-requests).

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1132670] Review Request: rubygem-compass-core - The Compass core stylesheet library

[bugzilla]

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

Jon Ciesla limburg...@gmail.com changed:

   What|Removed |Added

  Flags|fedora-cvs? |fedora-cvs+



-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1132670] Review Request: rubygem-compass-core - The Compass core stylesheet library

[bugzilla]

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



--- Comment #7 from Mo Morsi mmo...@redhat.com ---
(In reply to Vít Ondruch from comment #6)
 * Test suite
   - I'd suggest to execute the test suite using following command:
 
 ruby -Ilib:test/units \
   -e 'Dir.glob ./test/units/*_test.rb, method(:require)'

Updated

 
   - Please expand the test suite in %check section. This way you keep in RPM
 just the content which was originally shipped by upstream. This also
 avoids
 several rpmlint complains.

Assuming this meant exclude the test suite in the %files section. In which case
done.

   - It seems that the timecop and true are needed just for integration
 tests. Since there is circular dependency between true and compass, I'd
 suggest to use boostrap macro [1] to disable the integration tests during
 bootstrap.

Currently the integration tests are not being run anyways so just ommitting
this extra bit since its not needed. Once true is in we can update the spec to
run those tests.

 
 Otherwise the package is legit = APPROVED
 

Thanks

Final spec: https://mmorsi.fedorapeople.org/staging/rubygem-compass-core.spec
Final srpm:
https://mmorsi.fedorapeople.org/staging/rubygem-compass-core-1.0.1-4.fc22.src.rpm
Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=8467442

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1132670] Review Request: rubygem-compass-core - The Compass core stylesheet library

[bugzilla]

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

Mo Morsi mmo...@redhat.com changed:

   What|Removed |Added

  Flags||fedora-cvs?



--- Comment #8 from Mo Morsi mmo...@redhat.com ---
New Package SCM Request
===
Package Name: rubygem-compass-core
Short Description:  The Compass core stylesheet library
Upstream URL: http://rubygems.org/gems/compass-core
Owners: mmorsi
Branches: f20 f21
InitialCC:

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1132670] Review Request: rubygem-compass-core - The Compass core stylesheet library

[bugzilla]

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

Vít Ondruch vondr...@redhat.com changed:

   What|Removed |Added

  Flags|fedora-review?  |fedora-review+



--- Comment #6 from Vít Ondruch vondr...@redhat.com ---
* Test suite
  - I'd suggest to execute the test suite using following command:

ruby -Ilib:test/units \
  -e 'Dir.glob ./test/units/*_test.rb, method(:require)'

  - Please expand the test suite in %check section. This way you keep in RPM
just the content which was originally shipped by upstream. This also avoids
several rpmlint complains.
  - It seems that the timecop and true are needed just for integration
tests. Since there is circular dependency between true and compass, I'd
suggest to use boostrap macro [1] to disable the integration tests during
bootstrap.

Otherwise the package is legit = APPROVED


[1] https://fedoraproject.org/wiki/Packaging:Guidelines#Bootstrapping

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1132670] Review Request: rubygem-compass-core - The Compass core stylesheet library

[bugzilla]

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

Mo Morsi mmo...@redhat.com changed:

   What|Removed |Added

 Blocks||1152676




Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1152676
[Bug 1152676] installed rubygem-compass and rubygem-sass are incompatible.
-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1132670] Review Request: rubygem-compass-core - The Compass core stylesheet library

[bugzilla]

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

Mo Morsi mmo...@redhat.com changed:

   What|Removed |Added

 Depends On||1142508



--- Comment #5 from Mo Morsi mmo...@redhat.com ---
Spec: https://mmorsi.fedorapeople.org/staging/rubygem-compass-core.spec
SRPM:
http://mmorsi.fedorapeople.org/staging/rubygem-compass-core-1.0.1-3.fc20.src.rpm

Updated to run tests. Updated test suite pulls in rubygem-true, which has been
seperately packaged / submitted. Minitest5 patch backported.


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1142508
[Bug 1142508] Review Request: rubygem-true - Testing framework for Sass
libraries
-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1132670] Review Request: rubygem-compass-core - The Compass core stylesheet library

[bugzilla]

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



--- Comment #4 from Vít Ondruch vondr...@redhat.com ---
(In reply to Mo Morsi from comment #3)
 The upstream compass test suite isn't minitest5 compatible yet, filed a few
 PR's and will update the latest to the most recent master shortly. After
 that we can backport the updates to 1.0.1 and should be able to include the
 test suite in compass.

Well there are several options how to execute the test suite:

1) Patch, including submitting it upstream.
2) Monkey patching, e.g. rubygem-backports [1]
3) BR: rubygem(minitest)  5


 See this issue regarding the md5 algorithm:
 
 https://github.com/Compass/compass/pull/1266

Thanks, I added comment there.

[1]
http://pkgs.fedoraproject.org/cgit/rubygem-backports.git/tree/rubygem-backports.spec#n55

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1132670] Review Request: rubygem-compass-core - The Compass core stylesheet library

[bugzilla]

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

Vít Ondruch vondr...@redhat.com changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
 CC||vondr...@redhat.com
   Assignee|nob...@fedoraproject.org|vondr...@redhat.com
  Flags||fedora-review?



--- Comment #1 from Vít Ondruch vondr...@redhat.com ---
I'll take this for a review.

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1132670] Review Request: rubygem-compass-core - The Compass core stylesheet library

[bugzilla]

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



--- Comment #2 from Vít Ondruch vondr...@redhat.com ---
* 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].

* md5 usage
  - it seems that Compass uses md5 in
lib/compass/core/sass_extensions/functions/files.rb . Since this hash
algorithm is considered insecure and it is not supported on FIPS compliant
machines, could you please query upstream about it? This is not blocker,
just NTH issue I noticed.


[1] https://github.com/Compass/compass/tree/stable/core

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1132670] Review Request: rubygem-compass-core - The Compass core stylesheet library

[bugzilla]

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



--- Comment #3 from Mo Morsi mmo...@redhat.com ---
Thx for review. Updated:

Spec: https://mmorsi.fedorapeople.org/staging/rubygem-compass-core.spec
SRPM:
http://mmorsi.fedorapeople.org/staging/rubygem-compass-core-1.0.1-2.fc20.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=7476096

The upstream compass test suite isn't minitest5 compatible yet, filed a few
PR's and will update the latest to the most recent master shortly. After that
we can backport the updates to 1.0.1 and should be able to include the test
suite in compass.

See this issue regarding the md5 algorithm:

https://github.com/Compass/compass/pull/1266

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review