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 - Review Request: nodejs-filelist - Lazy-evaluating list of files, based on globs or regexes
Summary: Review Request: nodejs-filelist - Lazy-evaluating list of files, based on glo...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1164481
Blocks: nodejs-reviews 1164484
TreeView+ depends on / blocked
 
Reported: 2014-11-15 17:28 UTC by Tom Hughes
Modified: 2014-12-06 10:44 UTC (History)
3 users (show)

Fixed In Version: nodejs-json-localizer-0.0.2-1.fc21
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-12-01 19:03:24 UTC
panemade: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

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.


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