Re: [PATCH] gnu: Add sdparm.

[Tobias Geerinckx-Rice <tobias.geerinckx.rice@gmail.com>]

On 2 February 2016 at 11:36, Ricardo Wurmus
<ricardo.wurmus@mdc-berlin.de> wrote:
>
> tobias.geerinckx.rice@gmail.com writes:
>
>> From: Tobias Geerinckx-Rice <tobias.geerinckx.rice@gmail.com>
>>
>> * gnu/packages/disk.scm (sdparm): New variable.
>> ---
>>  gnu/packages/disk.scm | 27 +++++++++++++++++++++++++++
>>  1 file changed, 27 insertions(+)
>
> This patch looks very good!  Thank you!
>
>> +    (description "Sdparm reads and modifies SCSI device parameters.
>> +These devices can be SCSI disks, in which case @command{sdparm}'s role is
>> +similar to its namesake: the Linux @command{hdparm} utility originally
>> +designed for ATA disks.  However, @command{sdparm} can be used to access
>> +parameters on any device that uses a SCSI command set.  Such devices
>> +include CD/DVD drives (irrespective of transport), SCSI and ATAPI tape
>> +drives, and SCSI enclosures.  A small set of commands associated with
>> +starting and stopping the media, loading and unloading removable media
>> +and some other housekeeping functions can also be sent with this utility.")
>
> I would remove “Linux” from the description as neither “sdparm” nor
> “hdparm” seem to have anything to do with the kernel.

This caught my suspicion as well, but then hdparm turned out to be in
linux.scm and I dwelt no further on the matter. Go ahead!

>
> “[...] functions can [...] be sent” sounds odd to me.  It probably is
> meant to be understood as “commands associated with [...] housekeeping
> functions”, but that’s not how I parsed it at first.  Maybe it would be
> clearer to reorder the sentence as “This utility can also send commands
> associated with [this, that, and] some other housekeeping functions”.
>
> If you’re okay with these changes I’ll push it after these slight
> modifications.

Both are certainly improvements. Thank you for your help!

> Thanks again and welcome!

I'm going to have to use Emacs, aren't I...

> ~~ Ricardo

T G-R