Bug 755602 - Review Request: json_diff - Generates diff between two JSON files
Summary: Review Request: json_diff - Generates diff between two JSON files
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: 2011-11-21 15:14 UTC by Matěj Cepl
Modified: 2018-04-11 16:37 UTC (History)
6 users (show)

Fixed In Version: json_diff-1.1.0-1.el5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-11-23 23:32:28 UTC
Type: ---
Embargoed:
panemade: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Matěj Cepl 2011-11-21 15:14:52 UTC
Spec URL: http://mcepl.fedorapeople.org/rpms/json_diff.spec
SRPM URL: http://mcepl.fedorapeople.org/rpms/json_diff-0.9.1-1.el6.src.rpm
Description:
Compares two JSON files (http://json.org) and generates a new JSON file
with the result. Allows exclusion of some keys from the comparison, or
in other way to include only some keys.

Comment 1 Parag AN(पराग) 2011-11-21 16:20:32 UTC
Review:

+ koji scratch build (f17) ->http://koji.fedoraproject.org/koji/taskinfo?taskID=3529825

+ rpmlint on rpms gave
json_diff.src:28: W: macro-in-comment %_
json_diff.src:33: W: macro-in-comment %_
json_diff.src:33: W: macro-in-comment %{buildroot}
json_diff.noarch: W: no-manual-page-for-binary json_diff
2 packages and 0 specfiles checked; 0 errors, 4 warnings.
==> Above is not a blocker but at least macro warning can be fixed using %% instead %

- source match with upstream as (sha1sum)
  Ok I got confused here. upstream have latest today's commit with 0.9.0 tag whereas packaged tarball is 0.9.1 version

+ rest looks fine

suggestions:
1) If building this on Fedora only then you don't need
  a) buidlroot
  b) cleaning of buildroot in %install
  c) %clean section
  d) defattr(-,root,root,-)

2) Python sitelib macro not needed now in Fedora

3) install command should preserve timestamp http://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps

Comment 2 Matěj Cepl 2011-11-21 18:52:25 UTC
(In reply to comment #1)

My koji build is http://koji.fedoraproject.org/koji/taskinfo?taskID=3530449

> ==> Above is not a blocker but at least macro warning can be fixed using %%
> instead %

All commented macros are now eliminated, they are actually used now.

> - source match with upstream as (sha1sum)
>   Ok I got confused here. upstream have latest today's commit with 0.9.0 tag
> whereas packaged tarball is 0.9.1 version

To make things even more confusing, it is now 0.9.2 (both in the upstream tag, at PyPI and in the package).

> suggestions:
> 1) If building this on Fedora only then you don't need
>   a) buidlroot
>   b) cleaning of buildroot in %install
>   c) %clean section
>   d) defattr(-,root,root,-)
> 
> 2) Python sitelib macro not needed now in Fedora

Package should be available even for EL-5.

> 3) install command should preserve timestamp
> http://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps

Just now using only python setup.py install.

.spec file is at the same URL and .src.rpm at
http://mcepl.fedorapeople.org/rpms/json_diff-0.9.2-1.el6.src.rpm

Comment 3 Parag AN(पराग) 2011-11-22 11:36:20 UTC
+ koji build -> http://koji.fedoraproject.org/koji/taskinfo?taskID=3531690

+ rpmlint on rpms is now
json_diff.noarch: W: no-manual-page-for-binary json_diff
2 packages and 0 specfiles checked; 0 errors, 1 warnings.

+ source verified with upstream as (shs1sum)
094324bb69e5ae45deb189efd996e30cf1bfbe7c  json_diff-0.9.2.tar.gz
094324bb69e5ae45deb189efd996e30cf1bfbe7c  ../SOURCES/json_diff-0.9.2.tar.gz


Suggestion:
1) you should write a wrapper script in /usr/bin instead of linking json_diff.py in /usr/bin/json_diff

see examples files createrepo or system-config-printer in /usr/bin

Comment 4 Matěj Cepl 2011-11-22 14:36:17 UTC
(In reply to comment #3)
> Suggestion:
> 1) you should write a wrapper script in /usr/bin instead of linking
> json_diff.py in /usr/bin/json_diff
> 
> see examples files createrepo or system-config-printer in /usr/bin

Why? And how does it break Packaging Guidelines?

Comment 5 Parag AN(पराग) 2011-11-22 15:09:14 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Suggestion:
> > 1) you should write a wrapper script in /usr/bin instead of linking
> > json_diff.py in /usr/bin/json_diff
> > 
> > see examples files createrepo or system-config-printer in /usr/bin
> 
> Why? And how does it break Packaging Guidelines?

Fine. That was just a suggestion.

APPROVED.

Comment 6 Matěj Cepl 2011-11-22 16:51:02 UTC
New Package SCM Request
=======================
Package Name: json_diff
Short Description: Generates diff between two JSON files
Owners: mcepl
Branches: f16 el6 el5
InitialCC:

Comment 7 Gwyn Ciesla 2011-11-22 17:04:16 UTC
Git done (by process-git-requests).

Comment 8 Fedora Update System 2011-11-22 19:29:21 UTC
json_diff-0.9.2-2.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/json_diff-0.9.2-2.fc16

Comment 9 Fedora Update System 2011-11-22 19:29:51 UTC
json_diff-0.9.2-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/json_diff-0.9.2-2.el6

Comment 10 Fedora Update System 2011-11-22 19:30:25 UTC
json_diff-0.9.2-2.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/json_diff-0.9.2-2.el5

Comment 11 Fedora Update System 2011-11-23 23:32:28 UTC
json_diff-0.9.2-2.fc16 has been pushed to the Fedora 16 stable repository.

Comment 12 Fedora Update System 2011-11-29 19:36:21 UTC
json_diff-1.1.0-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/json_diff-1.1.0-1.el6

Comment 13 Fedora Update System 2011-11-29 19:42:41 UTC
json_diff-1.1.0-1.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/json_diff-1.1.0-1.el5

Comment 14 Fedora Update System 2011-12-16 17:42:19 UTC
json_diff-1.1.0-1.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 15 Fedora Update System 2011-12-16 17:42:27 UTC
json_diff-1.1.0-1.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.


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