Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.

Bug 1164483

Summary: Review Request: nodejs-filelist - Lazy-evaluating list of files, based on globs or regexes
Product: [Fedora] Fedora Reporter: Tom Hughes <tom>
Component: Package ReviewAssignee: Parag AN(पराग) <panemade>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: package-review, panemade, tom
Target Milestone: ---Flags: panemade: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: nodejs-json-localizer-0.0.2-1.fc21 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-12-01 19:03:24 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On: 1164481    
Bug Blocks: 956806, 1164484    

Description Tom Hughes 2014-11-15 17:28:27 UTC
Spec URL: http://download.compton.nu/nodejs/nodejs-filelist.spec
SRPM URL: http://download.compton.nu/nodejs/nodejs-filelist-0.0.3-1.fc20.src.rpm
Fedora Account System Username: tomh

Description:
A FileList is a lazy-evaluated list of files. When given a list of
glob patterns for possible files to be included in the file list,
instead of searching the file structures to find the files, a FileList
holds the pattern for latter use.

This allows you to define a FileList to match any number of files, but
only search out the actual files when then FileList itself is actually
used. The key is that the first time an element of the FileList/Array
is requested, the pending patterns are resolved into a real list of
file names.

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

+ Package built successful in mock (f22 x86_64)

+ rpmlint on generated rpms gave output
nodejs-filelist.noarch: W: spelling-error Summary(en_US) regexes -> regexps, reg exes, reg-exes
nodejs-filelist.noarch: W: only-non-binary-in-usr-lib
nodejs-filelist.noarch: W: dangling-symlink /usr/lib/node_modules/filelist/node_modules/minimatch /usr/lib/node_modules/minimatch
nodejs-filelist.noarch: W: dangling-symlink /usr/lib/node_modules/filelist/node_modules/utilities /usr/lib/node_modules/utilities
nodejs-filelist.src: W: spelling-error Summary(en_US) regexes -> regexps, reg exes, reg-exes
2 packages and 0 specfiles checked; 0 errors, 5 warnings.

+ Source0 verified with upstream as (sha256sum)
srpm tarball: b1c7d61ab5cea2f75d290c9cba3f418ed3debc7bd19c4bcb1e4b8e09287693cb
upstream tarball:b1c7d61ab5cea2f75d290c9cba3f418ed3debc7bd19c4bcb1e4b8e09287693cb

+ License is "ASL 2.0" and included in separate its own text file.

+ follows nodejs packaging guidelines.

Suggestions:
1) I think you should write tests in %check and not based on conditional execute %check. So,

%if 0%{?enable_tests}
%check
%nodejs_symlink_deps --check
jake test
%endif

should be

%check
%if 0%{?enable_tests}
%nodejs_symlink_deps --check
jake test
%endif

APPROVED.

Comment 2 Tom Hughes 2014-11-20 08:18:51 UTC
New Package SCM Request
=======================
Package Name: nodejs-filelist
Short Description: Lazy-evaluating list of files, based on globs or regexes
Upstream URL: https://github.com/mde/filelist
Owners: tomh
Branches: f20 f21
InitialCC: jamielinux

Comment 3 Gwyn Ciesla 2014-11-20 13:41:49 UTC
Git done (by process-git-requests).

Comment 4 Parag AN(पराग) 2014-11-20 15:08:14 UTC
hey, no reply on my suggestion and committed same spec??

Comment 5 Tom Hughes 2014-11-20 15:16:53 UTC
Sorry, didn't realise you were expecting a response.

I can make that change if you like - not sure why you think one way is better than the other though? That conditional can go away anyway once jake is in - it's only there to break the dependency loop that the tests require jake but this is a dependency of jake.

Thanks for the reviews by the way.

Comment 6 Parag AN(पराग) 2014-11-20 15:32:39 UTC
Thank you for detailed reply. Just wanted to know if there is any specific reason as I am used to with other way. But then no issues :)

Comment 7 Fedora Update System 2014-11-20 15:57:08 UTC
nodejs-json-localizer-0.0.2-1.fc21,nodejs-filelist-0.0.3-1.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/nodejs-json-localizer-0.0.2-1.fc21,nodejs-filelist-0.0.3-1.fc21

Comment 8 Fedora Update System 2014-11-20 15:57:09 UTC
nodejs-json-localizer-0.0.2-1.fc20,nodejs-filelist-0.0.3-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/nodejs-json-localizer-0.0.2-1.fc20,nodejs-filelist-0.0.3-1.fc20

Comment 9 Fedora Update System 2014-11-22 12:43:10 UTC
nodejs-json-localizer-0.0.2-1.fc20, nodejs-filelist-0.0.3-1.fc20 has been pushed to the Fedora 20 testing repository.

Comment 10 Fedora Update System 2014-12-01 19:03:24 UTC
nodejs-json-localizer-0.0.2-1.fc20, nodejs-filelist-0.0.3-1.fc20 has been pushed to the Fedora 20 stable repository.

Comment 11 Fedora Update System 2014-12-06 10:44:54 UTC
nodejs-json-localizer-0.0.2-1.fc21, nodejs-filelist-0.0.3-1.fc21 has been pushed to the Fedora 21 stable repository.