Bug 553186
| Summary: | Review Request: jansson - JSON parsing library | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Sean Middleditch <sean> |
| Component: | Package Review | Assignee: | Michael Schwendt <bugs.michael> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | bugs.michael, fedora-package-review, jiri, jsmith.fedora, jv+fedora, notting |
| Target Milestone: | --- | Flags: | bugs.michael:
fedora-review+
jv+fedora: needinfo? gwync: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | 1.1.3-4.fc12 | Doc Type: | Bug Fix |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2010-01-14 01:07:46 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
Sean Middleditch
2010-01-07 11:04:29 UTC
* The pkgconfig patch is acceptable. Though, simple libraries like this one (empty cflags, trivial ldflags) can be checked with autotools macros. * There is a nasty autoheader warning. A simple "touch config.h.in" in %prep fixes it here. * A shorter .bz2 tarball is available upstream. * If you don't plan to maintain this for EPEL, you could simplify the spec file in several places. Are you aware of that possibility? * The short "README" file is very confusing as it refers to something that isn't possible with your package. * No API documentation. Not even the .rst files are included. Can the html doc be generated within Fedora? Or could you mirror a snapshot of the online docs and use them in a second Source archive? * The %changelog refers to the future: Thu Dec 07 2010 * Why is "make check" not run? It looks suitable for a %check section. And even if it didn't work or didn't return compatible error codes, it's common practise to run it in %check to fill the build logs with as many test results as possible. Thanks for the review Michael. In order: * I don't use autotools for everything, and neither does everyone else. pkg-config makes it way, way easier when you aren't using autotools. You an even put the pkg-config lines directly into a simple Makefile. * Fixed. * Fixed. * I was not aware, but I'm not sure I won't maintain for EPEL. Only reason I'm not putting at least one of my other packages in EPEL already is that I don't have a personal-use CentOS server anymore, but that will be resolved. * Should I write a custom README or just drop it, then? I've seen plenty of other packages with READMEs that contain unnecessary/unusable source installation instructions. * Fedora has the Sphinx package for generating the docs. I added the proper BuildRequires on python-sphinx, added the lines to %build to build the docs, and added them as %doc files in the -devel package. * Ha, whoops. My brain caught up to 2010 but not the change in month. Go figure. Fixed. (rpmlint should probably catch this, but doesn't) * Wasn't aware there even was a %check section. Added. Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1911902 Updated Spec: http://middleditch.us/sean/fedora/jansson/jansson.spec Updated SRPRM: http://middleditch.us/sean/fedora/jansson/jansson-1.1.3-2.fc13.src.rpm That did it. Except for the %doc doc/README file which is still useless. "Plenty of other packages" are poor examples, unfortunately. Better drop it because its contents don't add any value at all. If you're concerned about dropping a %doc file which may change to include something useful in future upgrades, you could be smart and add a guard somewhere in %prep which checks for a minimum size [or contents] of the file in the tarball and terminates the build if a certain condition is met. APPROVED New Package CVS Request ======================= Package Name: jansson Short Description: C library for encoding, decoding and manipulating JSON data Owners: elanthis Branches: F-12 F-13 CVS done (by process-cvs-requests.py) Note: You requested an F-13 branch, which has been ignored as we've not yet started early branching. jansson-1.1.3-4.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/jansson-1.1.3-4.fc12 jansson-1.1.3-4.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report. Package Change Request ====================== Package Name: jansson New Branches: el6 Owners: jirka kanarip jsmith Git done (by process-git-requests). I would like to see this package in EPEL 7. And I'm also willing to help maintaining the package. @jirka @kanarip @jsmith, are you OK with that? I intend to request a new branch for EPEL7. The package is in CentOS base. I missed that. Sorry for the noise. Package Change Request ====================== Package Name: jansson New Branches: epel7 Owners: jirka kanarip jsmith Git done (by process-git-requests). I'm not sure if this was right. The package is already present in EPEL. https://fedoraproject.org/wiki/EPEL: "EPEL packages are usually based on their Fedora counterparts and will never conflict with or replace packages in the base Enterprise Linux distributions." typo... The package is already present in RHEL. |