[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 --- Comment #23 from Fedora Update System upda...@fedoraproject.org 2011-03-02 19:54:21 EST --- rootplot-2.2.1-3.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Fixed In Version|rootplot-2.2.1-3.el5|rootplot-2.2.1-3.el6 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 --- Comment #22 from Fedora Update System upda...@fedoraproject.org 2011-02-15 16:54:00 EST --- rootplot-2.2.1-3.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Fixed In Version|rootplot-2.2.1-3.fc14 |rootplot-2.2.1-3.el5 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 --- Comment #21 from Fedora Update System upda...@fedoraproject.org 2011-02-14 13:11:37 EST --- rootplot-2.2.1-3.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/rootplot-2.2.1-3.el6 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 Steve Traylen steve.tray...@cern.ch changed: What|Removed |Added Flag|fedora-cvs+ |fedora-cvs? --- Comment #18 from Steve Traylen steve.tray...@cern.ch 2011-01-30 12:47:46 EST --- Package Change Request == Package Name: rootplot New Branches: el5 Owners: stevetraylen p.s I noticed that el5 is branched from devel where as el6 is branched from f12 which does not obviously make sense. Maybe both should come from devel now. Steve. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 --- Comment #19 from Dennis Gilmore den...@ausil.us 2011-01-30 19:00:27 EST --- Git done (by process-git-requests). -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 --- Comment #20 from Fedora Update System upda...@fedoraproject.org 2011-01-30 21:39:24 EST --- rootplot-2.2.1-3.el5 has been submitted as an update for Fedora EPEL 5. https://admin.fedoraproject.org/updates/rootplot-2.2.1-3.el5 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 --- Comment #16 from Fedora Update System upda...@fedoraproject.org 2010-12-01 16:55:30 EST --- rootplot-2.2.1-3.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|ON_QA |CLOSED Fixed In Version||rootplot-2.2.1-3.fc13 Resolution||ERRATA Last Closed||2010-12-01 16:55:36 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 --- Comment #17 from Fedora Update System upda...@fedoraproject.org 2010-12-01 17:01:07 EST --- rootplot-2.2.1-3.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Fixed In Version|rootplot-2.2.1-3.fc13 |rootplot-2.2.1-3.fc14 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 --- Comment #12 from Jason Tibbitts ti...@math.uh.edu 2010-11-23 11:00:49 EST --- Git done (by process-git-requests). -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 --- Comment #13 from Fedora Update System upda...@fedoraproject.org 2010-11-23 11:23:13 EST --- rootplot-2.2.1-3.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/rootplot-2.2.1-3.fc14 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|ASSIGNED|MODIFIED -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 --- Comment #14 from Fedora Update System upda...@fedoraproject.org 2010-11-23 11:23:19 EST --- rootplot-2.2.1-3.fc13 has been submitted as an update for Fedora 13. https://admin.fedoraproject.org/updates/rootplot-2.2.1-3.fc13 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|MODIFIED|ON_QA --- Comment #15 from Fedora Update System upda...@fedoraproject.org 2010-11-23 16:54:37 EST --- rootplot-2.2.1-3.fc13 has been pushed to the Fedora 13 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update rootplot'. You can provide feedback for this update here: https://admin.fedoraproject.org/updates/rootplot-2.2.1-3.fc13 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 Steve Traylen steve.tray...@cern.ch changed: What|Removed |Added Flag||fedora-cvs? --- Comment #11 from Steve Traylen steve.tray...@cern.ch 2010-11-22 17:31:48 EST --- New Package SCM Request === Package Name: rootplot Short Description: Plots ROOT data with matplotlib Owners: stevetraylen Branches: f13 f14 el6 InitialCC: I'll make a el5 if when root gets to el5. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=608332
--- Comment #10 from Steve Traylen steve.tray...@cern.ch 2010-11-21 14:18:22
EST ---
Interesting I had always assumed that
%{python_sitelib}/rootplot
or
%{python_sitelib}/rootplot/
were different but you are correct the second one does include the directory
as well.
Have opted for
%{python_sitelib}/rootplot/
for the same reasons as you suggest.
http://cern.ch/straylen/rpms/rootplot/rootplot.spec
http://cern.ch/straylen/rpms/rootplot/rootplot-2.2.1-2.fc13.src.rpm
Will request in a couple of days.
Thanks again.
Steve.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=608332
--- Comment #9 from Michael J Gruber michaeljgruber+fedoraproj...@gmail.com
2010-11-20 11:10:07 EST ---
(In reply to comment #8)
.. does this even build? You're missing the directory
%{python_sitelib}/rootplot_scripts/
along with its contents.
rootplot 2.2 has a different (simpler, better) directory structure, there's
only rootplot/ and the egg.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=608332
--- Comment #6 from Michael J Gruber michaeljgruber+fedoraproj...@gmail.com
2010-11-19 03:28:53 EST ---
(In reply to comment #5)
(In reply to comment #4)
I've heard someone say
- The %files section needs improvement. The statement
%{python_sitearch}/
ends up owning the system python directory, which is not wanted.
Is this different?
You had
%{python_sitearch}/
which indeed ends up owning the system python directory. Here Steve has
%{python_sitearch}/*
which only ends up owning whatever is in the system python directory of the
buildroot.
The star makes the difference. Thanks for the clarification!
--
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=608332
--- Comment #7 from Steve Traylen steve.tray...@cern.ch 2010-11-19 16:34:57
EST ---
Michael and Jussi, all very valuable, thankyou.
I've tried to comment on everything:
__ONE__
NOT OK specfile is properly named, is cleanly written and uses macros
consistently.
This was use of $RPM_BUILD_ROOT consistently but also use of %{_bindir}
that was questioned.
As was commented this is fine unlike using $RPM_BUILD_ROOT and %{buildroot}.
I have change %{} everywhere just as it is tidier I agree.
__TWO__
NOT OK latest version is being packaged.
I have updated to 2.2.1
__THREE__
You can use python directly.
I could for sure,... but the reason for choosing %{__python} is that
I hope to offer the package on EPEL5 with python26 pending root appearing
for python26 within EPEL5 and then this will make it easier to build
a secondary module for python26.
__FOUR__
Also, running the first example
http://packages.python.org/rootplot/plot_directive/pyplots/first.py
through python gives
Traceback (most recent call last):
File first.py, line 1, in module
import rootplot.root2matplotlib as r2m
ImportError: No module named rootplot.root2matplotlib
As commented this has been corrected with latest upstream.
__FIVE__
Still, IMHO using wildcards when they are not absolutely necessary is a bit of
bad style; for instance you won't notice if e.g. egg-info isn't built for some
reason. I highly recommend being more verbose, i.e. using
%{python_sitelib}/root2matplotlib/
%{python_sitelib}/rootplot_scripts/
%{python_sitelib}/rootplot-%{version}-py*.egg-info
instead of the needlessly general wildcard glob.
Okay I agree using
%{python_sitelib}/*
is lazy.
I have gone for a middle ground of
%{python_sitelib}/rootplot-%{version}-*
%dir %{python_sitelib}/rootplot
%{python_sitelib}/rootplot/*
and all these name spaces are definitely mine as it were.
__SIX__
Patch0 is missing a comment in the spec file. Please document
its purpose.
Done.
# Removes some #!/usr/bin/env python from some python libs.
Patch0: rootplot-rm-shbangs.patch
__SEVEN__
I'm not sure why you want to use the conditionals in
%if ! (0%{?fedora} 12 || 0%{?rhel} 5)
%{!?python_sitelib: %global python_sitelib %(%{__python} -c from
distutils.sysconfig import get_python_lib; print(get_python_lib()))}
%endif
I add conditionals since it's then obvious that in the future when
RHEL5 and FEDORA12 are dead and berried this can be removed completely.
... I'm keeping it.
http://cern.ch/straylen/rpms/rootplot/rootplot.spec
http://cern.ch/straylen/rpms/rootplot/rootplot-2.2.1-1.fc13.src.rpm
are new packages, I'll leave them here for a week or so before requesting
GIT in case you have any comments.
Thanks again, Steve.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=608332
--- Comment #8 from Jussi Lehtola jussi.leht...@iki.fi 2010-11-19 17:05:45
EST ---
(In reply to comment #7)
__FIVE__
Still, IMHO using wildcards when they are not absolutely necessary is a bit
of
bad style; for instance you won't notice if e.g. egg-info isn't built for
some
reason. I highly recommend being more verbose, i.e. using
%{python_sitelib}/root2matplotlib/
%{python_sitelib}/rootplot_scripts/
%{python_sitelib}/rootplot-%{version}-py*.egg-info
instead of the needlessly general wildcard glob.
Okay I agree using
%{python_sitelib}/*
is lazy.
I have gone for a middle ground of
%{python_sitelib}/rootplot-%{version}-*
%dir %{python_sitelib}/rootplot
%{python_sitelib}/rootplot/*
and all these name spaces are definitely mine as it were.
.. does this even build? You're missing the directory
%{python_sitelib}/rootplot_scripts/
along with its contents.
As a stylistic issue, I don't see any point in using
%dir %{python_sitelib}/rootplot
%{python_sitelib}/rootplot/*
since both
%{python_sitelib}/rootplot
or
%{python_sitelib}/rootplot/
have the same function. I prefer the latter version, which demonstrates that
this is a directory.
I add conditionals since it's then obvious that in the future when
RHEL5 and FEDORA12 are dead and berried this can be removed completely.
... I'm keeping it.
OK. I'm just saying nothing breaks even if you don't use the conditionals at
all and just declare the macro as usual.
RHEL5 is going to be around for a long time, still. F12 is practically dead
already, it's EOLin in a couple of weeks.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=608332
Michael J Gruber m...@fedoraproject.org changed:
What|Removed |Added
CC||m...@fedoraproject.org
Bug 608332 depends on bug 542990, which changed state.
Bug 542990 Summary: Review Request: root - Numerical data analysis framework
https://bugzilla.redhat.com/show_bug.cgi?id=542990
What|Old Value |New Value
Status|NEW |ASSIGNED
Status|ASSIGNED|CLOSED
Resolution||ERRATA
--- Comment #1 from Michael J Gruber m...@fedoraproject.org 2010-11-18
08:03:00 EST ---
Informal review:
OK source files match upstream:
5095f72025132711c472b5a0db6417be13492b6ce09a74dfea9afe27165a6000
rootplot-1.1.tar.gz
OK package meets naming and versioning guidelines.
NOT OK specfile is properly named, is cleanly written and uses macros
consistently.
OK dist tag is present.
OK build root is correct.
OK license field matches the actual license.
OK license is open source-compatible.
license text not included upstream.
NOT OK latest version is being packaged.
OK BuildRequires are proper.
OK %clean is present.
OK package builds in koji.
http://koji.fedoraproject.org/koji/taskinfo?taskID=2608031
OKpackage installs properly.
OK rpmlint is silent.
(3 spellcheck warnings which can be ignored.)
OK final provides and requires are sane:
rpm -q --provides rootplot
rootplot = 1.1-1.fc14
pm -q --requires rootplot
/usr/bin/python
numpy
python(abi) = 2.7
python-matplotlib
root-python
rpmlib(CompressedFileNames) = 3.0.4-1
rpmlib(FileDigests) = 4.6.0-1
rpmlib(PartialHardlinkSets) = 4.0.4-1
rpmlib(PayloadFilesHavePrefix) = 4.0-1
rpmlib(PayloadIsXz) = 5.2-1
N/A %check is present and all tests pass:
N/A no shared libraries are added to the regular linker search paths.
OK owns the directories it creates.
NOT OK doesn't own any directories it shouldn't.
OK no duplicates in %files.
OK file permissions are appropriate.
OK no scriptlets present.
OK documentation is small, so no -docs subpackage is necessary.
OK %docs are not necessary for the proper functioning of the package.
OK no headers.
OK no pkgconfig files.
OK no libtool .la droppings.
N/A desktop files valid and installed properly.
So the remaining issues are:
- Use macros rather than vars consistently (e.g. buildroot).
- You can use python directly.
- The latest upstream version is 2.2.
- Use %{python_sitelib}/root2matplotlib etc. so that you do not own all of
sitelib.
Also, running the first example
http://packages.python.org/rootplot/plot_directive/pyplots/first.py
through python gives
Traceback (most recent call last):
File first.py, line 1, in module
import rootplot.root2matplotlib as r2m
ImportError: No module named rootplot.root2matplotlib
Adjusting the import line results in a segmentation violation, which may very
well be ROOT's fault.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 --- Comment #2 from Michael J Gruber m...@fedoraproject.org 2010-11-18 08:28:50 EST --- As a more constructive side remark: Rebuilding the rpms with current source (rootplot 2.2) and adjusting as necessary makes the mentioned example run without a glitch. So this will be OK with an updated spec. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=608332
Jussi Lehtola jussi.leht...@iki.fi changed:
What|Removed |Added
Status|NEW |ASSIGNED
CC||jussi.leht...@iki.fi
AssignedTo|nob...@fedoraproject.org|jussi.leht...@iki.fi
Flag||fedora-review+
--- Comment #3 from Jussi Lehtola jussi.leht...@iki.fi 2010-11-18 11:26:41
EST ---
Actually rpmlint output is
rootplot.noarch: E: explicit-lib-dependency python-matplotlib
rootplot.noarch: W: spelling-error Summary(en_US) matplotlib - matelote,
matrilocal, marplot
rootplot.noarch: W: spelling-error %description -l en_US pyROOT - pyrostat,
pyrope, pyrone
rootplot.noarch: W: spelling-error %description -l en_US matplotlib -
matelote, matrilocal, marplot
rootplot.noarch: W: no-manual-page-for-binary rootplot
rootplot.noarch: W: no-manual-page-for-binary tree2hists
rootplot.src: W: spelling-error Summary(en_US) matplotlib - matelote,
matrilocal, marplot
rootplot.src: W: spelling-error %description -l en_US pyROOT - pyrostat,
pyrope, pyrone
rootplot.src: W: spelling-error %description -l en_US matplotlib - matelote,
matrilocal, marplot
2 packages and 0 specfiles checked; 1 errors, 8 warnings.
All of these are OK, though. The explicit lib dependency is only relevant in
binary packages, where RPM automatically adds dependencies on the necessary
shared libraries.
(In reply to comment #1)
So the remaining issues are:
- Use macros rather than vars consistently (e.g. buildroot).
Actually, the spec file does not mix styles. The only thing this applies is
using $RPM_OPT_FLAGS vs %{optflags} and $RPM_BUILD_ROOT vs %{buildroot}. The
choice between these is a matter of personal preference.
- You can use python directly.
Yes, there's not much sense in using macros for standard stuff, such as
%{__rm}, %{__mv}, %{__mkdir}, %{__mkdir_p} and so on even if they are
implemented in RPM. Although, using them is not forbidden.
- The latest upstream version is 2.2.
- Use %{python_sitelib}/root2matplotlib etc. so that you do not own all of
sitelib.
This is not a problem per se, the rootplot package only ends up owning the
directories that are in %{python_sitelib} *in the build root*.
What ends up being owned by
%{python_sitelib}/*
is (on F14)
/usr/lib/python2.7/site-packages/root2matplotlib
/usr/lib/python2.7/site-packages/rootplot-1.1-py2.7.egg-info
/usr/lib/python2.7/site-packages/rootplot_scripts
Still, IMHO using wildcards when they are not absolutely necessary is a bit of
bad style; for instance you won't notice if e.g. egg-info isn't built for some
reason. I highly recommend being more verbose, i.e. using
%{python_sitelib}/root2matplotlib/
%{python_sitelib}/rootplot_scripts/
%{python_sitelib}/rootplot-%{version}-py*.egg-info
instead of the needlessly general wildcard glob.
- Patch0 is missing a comment in the spec file. Please document its purpose,
e.g.
# Patch to remove shbangs from Python library
Patch0: rootplot-rm-shbangs.patch
- I'm not sure why you want to use the conditionals in
%if ! (0%{?fedora} 12 || 0%{?rhel} 5)
%{!?python_sitelib: %global python_sitelib %(%{__python} -c from
distutils.sysconfig import get_python_lib; print(get_python_lib()))}
%endif
I'd scrap the %if stuff altogether.
***
Review:
MUST: The package does not yet exist in Fedora. The Review Request is not a
duplicate. OK
MUST: The spec file for the package is legible and macros are used
consistently. OK
MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the
Licensing Guidelines. OK
MUST: The License field in the package spec file must match the actual license.
OK
- License is MIT Modern style with sublicense
http://fedoraproject.org/wiki/Licensing/MIT
MUST: The sources used to build the package must match the upstream source, as
provided in the spec URL. OK
016f11836fb3764dd89dd5f0c290816b rootplot-1.1.tar.gz
016f11836fb3764dd89dd5f0c290816b ../SOURCES/rootplot-1.1.tar.gz
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. N/A
MUST: Optflags are used and time stamps preserved. OK
MUST: Packages containing shared library files must call ldconfig. N/A
MUST: A package must own all directories that it creates or require the package
that owns the directory. OK
MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. OK
MUST: Permissions on files must be set properly. OK
MUST: Large documentation files must go in a -doc subpackage. N/A
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=608332
--- Comment #4 from Michael J Gruber m...@fedoraproject.org 2010-11-18
11:42:14 EST ---
(In reply to comment #3)
Actually rpmlint output is
rootplot.noarch: E: explicit-lib-dependency python-matplotlib
rootplot.noarch: W: spelling-error Summary(en_US) matplotlib - matelote,
matrilocal, marplot
rootplot.noarch: W: spelling-error %description -l en_US pyROOT - pyrostat,
pyrope, pyrone
rootplot.noarch: W: spelling-error %description -l en_US matplotlib -
matelote, matrilocal, marplot
rootplot.noarch: W: no-manual-page-for-binary rootplot
rootplot.noarch: W: no-manual-page-for-binary tree2hists
rootplot.src: W: spelling-error Summary(en_US) matplotlib - matelote,
matrilocal, marplot
rootplot.src: W: spelling-error %description -l en_US pyROOT - pyrostat,
pyrope, pyrone
rootplot.src: W: spelling-error %description -l en_US matplotlib - matelote,
matrilocal, marplot
2 packages and 0 specfiles checked; 1 errors, 8 warnings.
On my side that was rpmlint on spec and srpm. Sorry.
- Use macros rather than vars consistently (e.g. buildroot).
Actually, the spec file does not mix styles. The only thing this applies is
using $RPM_OPT_FLAGS vs %{optflags} and $RPM_BUILD_ROOT vs %{buildroot}. The
choice between these is a matter of personal preference.
It uses different styles for different variables ($RPM_BUILD_ROOT vs.
%{_bindir}). If that's not a problem I'm fine with it.
- Use %{python_sitelib}/root2matplotlib etc. so that you do not own all of
sitelib.
This is not a problem per se, the rootplot package only ends up owning the
directories that are in %{python_sitelib} *in the build root*.
I've heard someone say
- The %files section needs improvement. The statement
%{python_sitearch}/
ends up owning the system python directory, which is not wanted.
Is this different?
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=608332
--- Comment #5 from Jussi Lehtola jussi.leht...@iki.fi 2010-11-18 12:02:27
EST ---
(In reply to comment #4)
(In reply to comment #3)
Actually, the spec file does not mix styles. The only thing this applies is
using $RPM_OPT_FLAGS vs %{optflags} and $RPM_BUILD_ROOT vs %{buildroot}. The
choice between these is a matter of personal preference.
It uses different styles for different variables ($RPM_BUILD_ROOT vs.
%{_bindir}). If that's not a problem I'm fine with it.
Yes, this is no problem. There's no shell variable declared by rpmbuild to
point to %{_bindir}. However, the functions of $RPM_BUILD_ROOT and %{buildroot}
are identical, and so are those of $RPM_OPT_FLAGS and %{optflags}.
I've heard someone say
- The %files section needs improvement. The statement
%{python_sitearch}/
ends up owning the system python directory, which is not wanted.
Is this different?
You had
%{python_sitearch}/
which indeed ends up owning the system python directory. Here Steve has
%{python_sitearch}/*
which only ends up owning whatever is in the system python directory of the
buildroot.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=608332 Steve Traylen steve.tray...@cern.ch changed: What|Removed |Added Depends on||542990 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
