Re: [Buildroot] [PATCH v2 1/1] package/ledmon: new package

From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: Maksim Kiselev <bigunclemax@gmail.com>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH v2 1/1] package/ledmon: new package
Date: Mon, 21 Aug 2023 13:11:59 +0200	[thread overview]
Message-ID: <20230821131159.46c0a6ff@windsurf> (raw)
In-Reply-To: <20230820095833.602364-2-bigunclemax@gmail.com>

Hello Maksim,

On Sun, 20 Aug 2023 12:58:33 +0300
Maksim Kiselev <bigunclemax@gmail.com> wrote:

> Enclosure LED Utilities
> 
> ledmon and ledctl are userspace tools designed to control storage
> enclosure LEDs. The user must have root privileges to use these tools.
> 
> These tools use the SGPIO and SES-2 protocols to monitor and control LEDs.
> They been verified to work with Intel(R) storage controllers (i.e. the
> Intel(R) AHCI controller) and have not been tested with storage controllers of
> other vendors (especially SAS/SCSI controllers).
> 
> For backplane enclosures attached to ISCI controllers, support is limited to
> Intel(R) Intelligent Backplanes.
> 
> Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>

Thanks, I've applied to our next branch, with some very small changes. See below.

> +Date: Mon, 15 May 2023 14:46:56 +0300
> +Subject: [PATCH 1/2] Replace nonstandard on_exit() function by atexit()
> +
> +Date: Mon, 15 May 2023 19:29:45 +0300
> +Subject: [PATCH 2/2] Fix unknown type name ‘ssize_t’ error

The patches in Buildroot should not have 1/2, 2/2, because this gets
quickly out of sync. And indeed: you have 3 patches for this package,
and these 1/2 and 2/2 are already wrong. So we ask people to generate
patches with "git format-patch -N". Note that you can run "make
check-package" which validates this kind of coding style issues.

> +LEDMON_VERSION = 0.97
> +LEDMON_SITE = $(call github,intel,ledmon,v$(LEDMON_VERSION))
> +LEDMON_DEPENDENCIES = host-pkgconf pciutils sg3_utils udev
> +LEDMON_LICENSE = GPL-2.0
> +LEDMON_LICENSE_FILES = COPYING
> +# The code base also include a COPYING.LIB file with the LGPL-2.1 text,
> +# and some source files are published under LGPL-2.1, but all of them are
> +# at some point linked with GPL-2.0 code, making the resulting binaries
> +# GPL-2.0 licensed

I've moved this comment above LEDMON_LICENSE, as we always put comments
*before* the code they correspond to.

> +LEDMON_AUTORECONF = YES

I've added a comment above this line that explains why it is needed.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot