Re: [PATCH] Add a module for retrieving SMBIOS information

[Prarit Bhargava <prarit@redhat.com>]

On 02/03/2015 01:41 PM, David Michael wrote:
> On Tue, Feb 3, 2015 at 10:04 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>> On 02/02/2015 02:26 PM, Prarit Bhargava wrote:
>>>
>>>
>>> On 02/02/2015 12:09 PM, David Michael wrote:
>>>> On Mon, Feb 2, 2015 at 6:20 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>>>>> On 02/01/2015 09:05 PM, David Michael wrote:
>>>>>> * grub-core/commands/i386/smbios.c: New file.
>>>>>> * grub-core/Makefile.core.def (smbios): New module.
>>>>>> * docs/grub.texi (smbios): New node.
>>>>>> (Command-line and menu entry commands): Add a menu entry for smbios.
>>>>>> ---
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> There was some interest on help-grub about supporting SMBIOS access
>>>>>> upstream.
>>>>>
>>>>> OOC, why?  Why would you need to do this?  I'm certainly not against doing this
>>>>> but just wondering exactly why you want to do this.
>>>>
>>>> The thread on grub-help asked about booting particular kernel versions
>>>> off a hot-pluggable drive based on the detected hardware, which this
>>>> would allow.
>>>>
>>>> I originally wrote it to change what options are available based on
>>>> whether a disk is being booted physically or virtually.  Since QEMU
>>>> makes it easy to add SMBIOS entries on the command line, I've also
>>>> been using it for random tweaks like showing a vga_text boot menu
>>>> instead of gfxterm when running QEMU with "-display curses".
>>>>
>>>
>>> Ah interesting David -- and good job on getting the efi.smbios stuff in there
>>> too as that's an easy thing to miss.  I'll take a closer look ...
>>>
>>
>> FWIW, I think it looks fine and it definitely has a valid use case.  I'd suggest
>> that you update the description with Rajat's comment.
> 
> Okay, to be clear, by "description" here do you mean to put the use
> case in the commit message?

Yes, I find it useful to note the use case.  Later on if someone wants to make a
change to the module we'll know exactly why it was created.

^^^ The above is IMO and has become a sort of standard for other projects.  The
maintainer here may not like it ... but a few extra sentences in the commit
message can't hurt anything ;)

> 
>> One odd thing in the patch (and it may be something weird on my end that I've
>> never seen before).  When I saved and applied your patch via 'git am', the patch
>> contained a few "^L" lines.
> 
> Yes, I used formfeeds to follow the GNU coding standards document when
> I first wrote the module.  I'll take them out of the updated patch.

Thanks!

P.

> 
> Thanks.
> 
> David
>