Bug 895592 - (syntaxhighlighter) Review Request: syntaxhighlighter - JavaScript syntax highlighter
Review Request: syntaxhighlighter - JavaScript syntax highlighter
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christopher Meng
Fedora Extras Quality Assurance
:
Depends On:
Blocks: Horde_Mime_Viewer
  Show dependency treegraph
 
Reported: 2013-01-15 10:38 EST by Remi Collet
Modified: 2013-07-03 16:53 EDT (History)
3 users (show)

See Also:
Fixed In Version: syntaxhighlighter-3.0.83-2.el6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-06-26 21:53:52 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
i: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Remi Collet 2013-01-15 10:38:29 EST
Spec URL: https://raw.github.com/remicollet/remirepo/3451052faf5e5db905d59f6dbb27994b4f5f1bfe/syntaxhighlighter/syntaxhighlighter.spec
SRPM URL: http://rpms.famillecollet.com/SRPMS/syntaxhighlighter-3.0.83-1.remi.src.rpm
Description: 
SyntaxHighlighter is a fully functional self-contained code
syntax highlighter developed in JavaScript.

Fedora Account System Username: remi
Comment 1 Christopher Meng 2013-06-16 02:27:41 EDT
Hmm...No javascript packaging guideline now, only a draft.

From fedora-review seems OK, but somethings maybe you should fix:

1) I found that mixture of tab and space in License and Source0 tag.

2) This library has its GitHub page, and the author has tagged the 3.0.83 version.

https://github.com/alexgorbatchev/SyntaxHighlighter/tags

3) Is this spec for EL only? I've seen many obsoleted lines and buildroot tag.

4) A little more blanks between sections. Not mandatory.

5) License: MIT or GPLv2. Why not "and"?
Comment 2 Remi Collet 2013-06-16 03:18:20 EDT
(In reply to Christopher Meng from comment #1)
> Hmm...No javascript packaging guideline now, only a draft.
> 
> From fedora-review seems OK, but somethings maybe you should fix:
> 
> 1) I found that mixture of tab and space in License and Source0 tag.

Agree, will fix it

> 2) This library has its GitHub page, and the author has tagged the 3.0.83
> version.
> 
> https://github.com/alexgorbatchev/SyntaxHighlighter/tags

Yes, but the github only provides the "sources" and we don't have (yet) needed stuff to run the build (npm, jake, ...)

In fact, "master" use node.js stuff, while old release 3.0.83 use phing. 
I will look if we can build from source (it seems we need lot of fix because it use a very old version... of course... 2010)

But don't know if it worth the work... as will be broken as soon as a new version will be released.

Build for JS is mostly: concat + compress (result still readable)

> 3) Is this spec for EL only? I've seen many obsoleted lines and buildroot
> tag.

This spec target EL-5, so yes buildroot, ... are required.

> 4) A little more blanks between sections. Not mandatory.
> 
> 5) License: MIT or GPLv2. Why not "and"?

Because https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines#Dual_Licensing_Scenarios
Comment 5 Christopher Meng 2013-06-16 07:03:52 EDT
cp %{SOURCE2} %{SOURCE3} .

should be:

cp -p %{SOURCE2} %{SOURCE3} .

And please preserve the ts when downloading these two licenses.

APPROVED.

Please FIX this problem before importing.
Comment 6 Christopher Meng 2013-06-16 07:16:02 EDT
Plus, no need for  %defattr(-,root,root,-), it's only needed < RPM 4.4
Comment 7 Remi Collet 2013-06-16 10:28:49 EDT
Thanks for the review.

I will clean the EL-5 stuff after import (EL-5 is no more a target as phing is missing)


New Package SCM Request
=======================
Package Name: syntaxhighlighter
Short Description: JavaScript syntax highlighter
Owners: remi
Branches: f18 f19 el6
InitialCC:
Comment 8 Gwyn Ciesla 2013-06-17 08:17:09 EDT
Git done (by process-git-requests).
Comment 9 Fedora Update System 2013-06-17 13:13:36 EDT
syntaxhighlighter-3.0.83-2.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/syntaxhighlighter-3.0.83-2.fc18
Comment 10 Fedora Update System 2013-06-17 13:13:51 EDT
syntaxhighlighter-3.0.83-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/syntaxhighlighter-3.0.83-2.el6
Comment 11 Fedora Update System 2013-06-17 13:14:01 EDT
syntaxhighlighter-3.0.83-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/syntaxhighlighter-3.0.83-2.fc19
Comment 12 Fedora Update System 2013-06-17 21:27:50 EDT
syntaxhighlighter-3.0.83-2.fc18 has been pushed to the Fedora 18 testing repository.
Comment 13 Fedora Update System 2013-06-26 21:53:52 EDT
syntaxhighlighter-3.0.83-2.fc18 has been pushed to the Fedora 18 stable repository.
Comment 14 Fedora Update System 2013-06-29 14:39:10 EDT
syntaxhighlighter-3.0.83-2.fc19 has been pushed to the Fedora 19 stable repository.
Comment 15 Fedora Update System 2013-07-03 16:53:50 EDT
syntaxhighlighter-3.0.83-2.el6 has been pushed to the Fedora EPEL 6 stable repository.

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