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

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
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.
	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: