Bug 505360

Summary: Review Request: JSCookMenu - Javascript GUI-like web menus
Product: [Fedora] Fedora Reporter: Patrick Monnerat <patrick>
Component: Package ReviewAssignee: Caius Chance <K9>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, kevin, notting, petersen
Target Milestone: ---Flags: K9: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: JSCookMenu-2.0.4-3.fc13 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-05-10 17:02:06 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:

Description Patrick Monnerat 2009-06-11 16:12:08 UTC
Spec URL: http://monnerat.fedorapeople.org/JSCookMenu.spec
SRPM URL: http://monnerat.fedorapeople.org/JSCookMenu-2.0.4-1.fc12.src.rpm

Description: JSCookMenu is a powerful menu script written in JavaScript that can
mimic complex menus found in popular GUI Applications. It is relatively
simple and easy to use. Creating a new theme requires some patience,
but rarely does one has to write one since some good ones are provided.
  The following features are implemented:
* Supports both horizontal and vertical menus.
* Supports relative positioning.
* Supports different menus with different themes in the same web page.
* Eases the menu creation process with a menu builder.
* Special effects such as sliding and fading in/out is available.
* APIs for JavaScript developers.

rpmlint output: JSCookMenu.noarch: W: no-documentation
--> No documentation files provided in the upstream tarball. The MIT-compatible license text is in script comment.
koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1405548

Comment 1 caius.chance 2010-01-05 07:07:48 UTC
Hi I'm here for prereview:

=====

$ rpmlint SPECS/JSCookMenu.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

=====

Please either include license file and mention in every source file, or include license contents in every source file.

Also, please persuade author to adopt MIT license rather than MIT-compatible license for legal concerns.

Comment 2 Patrick Monnerat 2010-01-14 14:57:09 UTC
The current license is OK: this is "MIT modern style with sublicense" https://fedoraproject.org/wiki/Licensing/MIT#Modern_Style_with_sublicense

Upstream request for license clarification has been submitted:
http://yuanheng.org/forum/viewtopic.php?t=1690

Comment 3 caius.chance 2010-01-14 17:16:18 UTC
Cool! I will follow up then.

Comment 4 Patrick Monnerat 2010-01-22 17:10:10 UTC
I got a reply on http://yuanheng.org/forum/viewtopic.php?t=1690

The theme files are public domain. Unfortunately this clarification only came on the forum page.

As you can see there, I'm insisting... hang on !

Comment 5 Patrick Monnerat 2010-01-25 13:42:48 UTC
License clarified by e-mail. E-mail added in %doc.
SRPM: http://monnerat.fedorapeople.org/JSCookMenu-2.0.4-2.fc12.src.rpm

rpmlint output:
JSCookMenu-2.0.4-2.fc12.src.rpm:
  none
JSCookMenu.spec:
  none
JSCookMenu-2.0.4-2.fc12.noarch.rpm:
  W: file-not-utf8 /usr/share/doc/JSCookMenu-2.0.4/JSCookMenu_theme_license_clarification.mail.txt
    The license clarification e-mail as downloaded contains an ISO-8859-1 part (with a single ISO8859-1 non-paddable space).

Comment 6 Caius Chance 2010-02-23 07:34:11 UTC
Great work Patrick! I have checked the .spec again and please improve the commands into Macro ones:

https://fedoraproject.org/wiki/Extras/ReferenceMandrakeRPMMacros

Comment 7 Patrick Monnerat 2010-02-23 10:51:24 UTC
Sorry to tell you, but I do not agree with this suggestion for the following reasons:
_ Although these macros are effectively defined in the /usr/lib/rpm/macros file, they are mainly intended to ease porting specs from the old Mandrake distribution. The 2 years-old page you reference to is annotated "This page could probably be deleted... Doesn't seem to be much useful information" and the referring page (https://fedoraproject.org/wiki/Packaging:RPMMacros#Reference) lists it only as a conversion aid from Mandrake spaces (altough not forbidding use of this macros, I conceed to you).
_ I did not find usage references to these macros in the packaging guidelines,  neither as a requirement nor even as a recommandation.
_ The use of these macros is not very frequent in current Fedora spec files.
_ Finally, I do not like them: they lower the spec file readability and they introduce a dependency to their definitions while it is unlikely that their defined value changes sometime.

Comment 8 Caius Chance 2010-02-24 07:02:55 UTC
MUST: Each package must consistently use macros.

I was misunderstood about this on review guidelines. Now I would interpret that as "either use that at highest degree or just don't use that at all". Given that you haven't used it almost. This should be recognized as proper in your .spec file.

Comment 9 Caius Chance 2010-02-24 07:08:58 UTC
The .spec and srpm looks okay to me, but you need to get a sponsor for this new package approved.

Comment 10 Patrick Monnerat 2010-02-24 09:24:13 UTC
I agree with your new opinion about macro usage consistency: if a particular macro (or macro "class") is used in a package, it must be used everywhere it can !

Thanks for reviewing. However I do not need a sponsor: I'm a Fedora developer for 18 months now! If you're still OK to accept this review, please set the flag accordingly.

Thanks

Comment 11 Caius Chance 2010-04-29 03:07:18 UTC
$ rpmlint SPECS/JSCookMenu.spec SRPMS/JSCookMenu-2.0.4-2.fc12.src.rpm 
JSCookMenu.src: W: spelling-error Summary(en_US) Javascript -> JavaScript, Java script, Java-script
1 packages and 1 specfiles checked; 0 errors, 1 warnings.

# Ignorable.

Comment 12 Caius Chance 2010-04-29 03:17:21 UTC
All MUST are passed.

Approved.

Comment 13 Patrick Monnerat 2010-04-30 09:15:40 UTC
Many thanks for your work on this, Kaio.

New Package CVS Request
=======================
Package Name: JSCookMenu 
Short Description: Javascript GUI-like web menus
Owners: monnerat
Branches: F-12 F-13
InitialCC:

Comment 14 Kevin Fenzi 2010-05-04 03:06:20 UTC
I can't seem to find the guideline now, but I don't think you should reload httpd in pre/post here. 

Think about a web server where the admin is installing some packages and a reload disrupts their server (or there are changes that are incomplete and it doesn't restart correctly). 

I would advise not restarting httpd here.

Comment 15 Patrick Monnerat 2010-05-04 17:04:31 UTC
Well... this is a philosophical problem that could well degenerate into a troll :-(

In the case you mention, you're probably right, but this implies system updates are done while someone changes the server configuration, saves the (bad) configuration and does not test/apply the changes immediately... IMHO, this situation will occur very seldom.

However, I'm not wrong because there is an /etc/httpd/conf.d/*.conf file in this package and a server reload is needed to enable/disable it. phpPgAdmin (for example) reloads the server at post time for the same reason. And by the way, this is just a reload, not a restart.

This ideal situation would be to have an Apache-enabled /usr/share/js directory for those kind of packages (a bit like /usr/share/php for php packages). This would allow new js packages to get rid of a specific httpd configuration file. But Fedora does not (yet) include such a directory in its standard base, nor there is specific guidelines for javascript packaging.

In the meantime, I suggest we meet halfway by reloading the server only at first package install and last uninstall. @Kevin: would it be satisfactory to you ?

Comment 16 Kevin Fenzi 2010-05-05 03:27:00 UTC
Well, the only packages in the distro that do this are: 

phpPgAdmin, postgresql-pgpoolAdmin and w3c-markup-validator.

There are 134 packages in rawhide that provide a /etc/httpd/conf.d/*.conf file. 

I would suggest that the 134 packages are doing the right thing here and the other 3 should get fixed to not do this. ;) 

I won't hold up your review any further, but I think consulting the packaging mailing list might be a good idea before importing?

Comment 17 Kevin Fenzi 2010-05-06 15:10:44 UTC
CVS done (by process-cvs-requests.py).

Comment 18 Fedora Update System 2010-05-07 13:37:06 UTC
JSCookMenu-2.0.4-3.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/JSCookMenu-2.0.4-3.fc12

Comment 19 Fedora Update System 2010-05-07 13:37:13 UTC
JSCookMenu-2.0.4-3.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/JSCookMenu-2.0.4-3.fc13

Comment 20 Patrick Monnerat 2010-05-07 13:52:30 UTC
Thanks Kevin.
Typo fixed and post/postun scriptlets removed.

Comment 21 Fedora Update System 2010-05-10 17:02:00 UTC
JSCookMenu-2.0.4-3.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 22 Fedora Update System 2010-05-10 23:48:59 UTC
JSCookMenu-2.0.4-3.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.