Bug 1099269 - Review Request: drupal7-admin_theme - Allows you to define a different theme for admin pages
Summary: Review Request: drupal7-admin_theme - Allows you to define a different theme ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-05-20 01:09 UTC by Sam Wilson
Modified: 2014-11-01 17:02 UTC (History)
5 users (show)

Fixed In Version: drupal7-admin_theme-1.0-2.fc21
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-10-22 08:52:14 UTC
Type: ---
Embargoed:
panemade: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Sam Wilson 2014-05-20 01:09:51 UTC
Spec URL: http://cycloptivity.fedorapeople.org/drupal7-admin_theme/1.0-1/drupal7-admin_theme.spec
SRPM URL: http://cycloptivity.fedorapeople.org/drupal7-admin_theme/1.0-1/drupal7-admin_theme-1.0-1.fc19.src.rpm
Description: Drupal allows you to define a different theme for administration pages. By
default this only applies to pages with a path starting with 'admin' and
content editing pages.
Fedora Account System Username: cycloptivity

Koji build http://koji.fedoraproject.org/koji/taskinfo?taskID=6865721

Comment 1 Parag AN(पराग) 2014-05-20 08:11:50 UTC
Review:

+ Package builds successful in F21 x86_64 mock

+ rpmlint on generated rpms gave
drupal7-admin_theme.noarch: W: spelling-error Summary(en_US) Drupal -> Drupe
drupal7-admin_theme.noarch: W: spelling-error %description -l en_US Drupal -> Drupe
drupal7-admin_theme.noarch: E: incorrect-fsf-address /usr/share/doc/drupal7-admin_theme/LICENSE.txt
drupal7-admin_theme.src: W: spelling-error Summary(en_US) Drupal -> Drupe
drupal7-admin_theme.src: W: spelling-error %description -l en_US Drupal -> Drupe
2 packages and 0 specfiles checked; 1 errors, 4 warnings.

+ Source verified with upstream as (sha256sum)
srpm tarball: ab3b742c9854ddad5862e197b17bcc2bc655a5f101df7ab31a055c0e7f25c00f
upstream tarball : ab3b742c9854ddad5862e197b17bcc2bc655a5f101df7ab31a055c0e7f25c00f

+ License is valid GPLv2+ and included in its own text file LICENSE.txt

+ Package follows drupal7 packaging guidelines

Suggestions:
1) Group tag is not necessary in Fedora now and can be removed from spec file. See https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag

2) Buildroot tag is not needed. See https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag

3) File upstream bug/ticket to correct the FSF address in LICENSE.txt

Comment 2 Shawn Iwinski 2014-05-20 18:34:55 UTC
(In reply to Parag AN(पराग) from comment #1)
> Suggestions:
> 1) Group tag is not necessary in Fedora now and can be removed from spec
> file. See https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag

Drupal 7 packaging guidelines template (https://fedoraproject.org/wiki/Packaging:Drupal7#Module) has the Group tag.  Per overall packaging guidelines (https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag) it can stay to be compatible with EPEL and all Drupal pkgs are packaged for EPEL.


> 2) Buildroot tag is not needed. See
> https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag

This is an EPEL5 item that needs to be removed unless planning to package for EPEL5 -- and in that case there are several other things missing.  I would suggest not packaging for EPEL5 unless you need it so drop the buildroot.


> 3) File upstream bug/ticket to correct the FSF address in LICENSE.txt

No upstream bug/ticket necessary because it will automatically get fixed on next upstream release by the Drupal buildbot that automatically adds the LICENSE.txt file -- the upstream module owner has no control over the LICENSE.txt file added.



Additional items:

4) "Requires: php(language) >= 4.1" not required and should not be present.  You do need to run phpcompatinfo on the module though -- see https://fedoraproject.org/wiki/Packaging:Drupal7#PHP_Extensions

5) This is a module, not a theme, so "%{module}" not "%{theme}"

Comment 3 Sam Wilson 2014-05-21 06:02:35 UTC
Spec URL: http://cycloptivity.fedorapeople.org/drupal7-admin_theme/1.0-2/drupal7-admin_theme.spec
SRPM URL: http://cycloptivity.fedorapeople.org/drupal7-admin_theme/1.0-2/drupal7-admin_theme-1.0-2.fc19.src.rpm

Thanks for the reviews guys.

@Parag
1. As Shawn pointed out this is from the Drupal guidelines
2. This is my mistake. The module SPEC template doesnt include a buildroot so thats been removed.
3. As per Shawn's update we discussed this on drupal-devel and its a bug that will be fixed via the Drupal release mechanism.

@Shawn
4. I missed that in the guidelines where its mentioned drupal7-rpmbuild handling the PHP lang dep. I've now removed it.

5. Pure failure on my part. theme in name != drupal theme :) Its now a module as expected.

Also, I will start running each scratch build against EPEL 6 rather than F21 since that's the intended target.

http://koji.fedoraproject.org/koji/taskinfo?taskID=6869385

Cheers,

Sam

Comment 4 Sam Wilson 2014-06-27 01:41:48 UTC
Hi Guys,

Any chance to review this updated package?

Cheers,

Sam

Comment 5 Parag AN(पराग) 2014-06-27 08:22:21 UTC
Thanks, the update in comment 3 looks good.

APPROVED.

Comment 6 Sam Wilson 2014-07-25 03:05:07 UTC
New Package SCM Request
=======================
Package Name: drupal7-admin_theme
Short Description: Drupal allows you to define a different theme for administration pages. By default this only applies to pages with a path starting with 'admin' and content editing pages.
Upstream URL: http://www.drupal.org/project/admin_theme
Owners: cycloptivity asrob siwinski 
Branches: f19 f20 el6 epel7
InitialCC:

Comment 7 Gwyn Ciesla 2014-07-25 11:59:23 UTC
Git done (by process-git-requests).

Comment 8 Fedora Update System 2014-10-13 11:51:54 UTC
drupal7-admin_theme-1.0-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/drupal7-admin_theme-1.0-2.fc20

Comment 9 Fedora Update System 2014-10-13 11:57:41 UTC
drupal7-admin_theme-1.0-2.el7 has been submitted as an update for Fedora EPEL 7.
https://admin.fedoraproject.org/updates/drupal7-admin_theme-1.0-2.el7

Comment 10 Fedora Update System 2014-10-13 12:14:30 UTC
drupal7-admin_theme-1.0-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/drupal7-admin_theme-1.0-2.fc19

Comment 11 Fedora Update System 2014-10-13 12:15:01 UTC
drupal7-admin_theme-1.0-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/drupal7-admin_theme-1.0-2.el6

Comment 12 Parag Nemade 2014-10-13 12:40:38 UTC
I got the permission from this package submitter to build his packages. I see this package missed f21 branch please create it.

Package Change Request
=======================
Package Name: drupal7-admin_theme
New Branches: f21
Owners: cycloptivity asrob siwinski

Comment 13 Fedora Update System 2014-10-13 21:41:31 UTC
drupal7-admin_theme-1.0-2.el6 has been pushed to the Fedora EPEL 6 testing repository.

Comment 14 Kevin Fenzi 2014-10-13 23:14:35 UTC
Git done (by process-git-requests).

Comment 15 Fedora Update System 2014-10-14 04:44:38 UTC
drupal7-admin_theme-1.0-2.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/drupal7-admin_theme-1.0-2.fc21

Comment 16 Fedora Update System 2014-10-22 08:52:14 UTC
drupal7-admin_theme-1.0-2.fc20 has been pushed to the Fedora 20 stable repository.

Comment 17 Fedora Update System 2014-10-22 08:53:25 UTC
drupal7-admin_theme-1.0-2.fc19 has been pushed to the Fedora 19 stable repository.

Comment 18 Fedora Update System 2014-10-28 10:56:41 UTC
drupal7-admin_theme-1.0-2.el7 has been pushed to the Fedora EPEL 7 stable repository.

Comment 19 Fedora Update System 2014-10-28 11:06:57 UTC
drupal7-admin_theme-1.0-2.el6 has been pushed to the Fedora EPEL 6 stable repository.

Comment 20 Fedora Update System 2014-11-01 17:02:01 UTC
drupal7-admin_theme-1.0-2.fc21 has been pushed to the Fedora 21 stable repository.


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