Re: [RFS] rocm-smi-lib/4.5.2-1~exp1
Hello Etienne,
On 2/15/22 23:32, Étienne Mollier wrote:
I brought a couple of modifications to the repository, so you
can refer to the git log if you are curious. Hereafter are some
of the items I thought might be worth being more verbose about.
Thank you!
d/control: I trimmed down the doxygen LaTeX dependencies to
doxygen-latex. If you're worried of your footprint on
public mirrors due to continuously downloading the same
heavy packages set again and again, you can investigate
the use of a caching proxy for the packages. I use
apt-cacher-ng, which has its quirks, but there are other
approaches.
Nice!
d/rules: git checkout is commented out so no harm done, but note
the package should not be assumed to be built from it's
source tree with VCS metadata. #991822 is a rather
extreme example of what could go wrong.
Right, cleaned it up.
d/copyright: this is a rather detailed d/copyright file. Maybe
it is possible to simplify it a bit by relying more on
wildcards? It looks otherwise accurate.
Done, hopefully right.
d/roc-smi.1: in the Copyright section, you may want to append
that you wrapped up the present manual page for the
Debian project (and that it is made available under
Expat license if I trust the copyright file, and can be
reused for other purposes). The plain "Copyright: AMD
All rights reserved" without further notice makes this
uneasy otherwise.
Same as above.
d/p/0001-Merged-python-rsmi-bindings-into-the-main-file.patch:
I'm a bit concerned by the mean used for inlining the
functions into roc-smi. The patch will require careful
maintenance at each upstream version to make sure it is
appropriately updated to mirror the content of the
initial file rsmiBindings.py. Not sure that will do
good to the package in the long run.
I haven't incorporated this bit of your review.
I am bad with python packaging and the dist-package thing.
There are incoming changes to rocm-smi-lib for the next ROCm release
anyway [1],
so if it's ok to change it next iteration, it feels good for me.
BR, Maxime
[1] https://lists.freedesktop.org/archives/amd-gfx/2022-February/075436.html
Reply to: