[Bug 608332] Review Request: rootplot - Plots ROOT data with matplotlib

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.lehtola@xxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]