All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pavel Roskin <proski@gnu.org>
To: The development of GRUB 2 <grub-devel@gnu.org>
Subject: Re: [PATCH] File access library for lua
Date: Tue, 23 Jun 2009 18:10:36 -0400	[thread overview]
Message-ID: <1245795036.3204.26.camel@mj> (raw)
In-Reply-To: <ca0f59980906230227y723b12afw8c825a7dbb160cf3@mail.gmail.com>

On Tue, 2009-06-23 at 17:27 +0800, Bean wrote:
> Hi,
> 
> Some bug fix for osdetect.lua, it also detect windows 98/me, freedos,
> msdos and freebsd.

Why FressDOS and FressBSD?  I assume it's typos.  Why isn't Linux
capitalized?  MS-DOS is written with a dash.  "Windows Vista bootmgr"
should be "Windows Vista" and "Windows NT/2000/XP loader" should be
"Windows NT/2000/XP".  It's not like we are just booting the loaders.

inird should be initrd.  Please add check for the Fedora style names for
initrd, namely "initrd-KVER.img".  Or maybe you just missed ".img" in
the second check?

> Extend the function of grub.file_exist to allow testing multiple names
> at the same time, this simplify osdetect.lua.

The change to grub_lua_file_exist() is dubious.  It's not clear why the
requirement is that all files exist.  Maybe I don't know the style of
lua, but I think it's wrong to hardcode the AND logic just because one
script would benefit from it.

If we consider e.g. the wildcard expansion in make, it will return a
non-empty value if any file exists, i.e. the OR logic is used.

I suggest that you split the lua.mod changes and osdetect.lua.  The
later is obviously a bikeshed issue that can be discussed for a long
time.  The former needs a more technical consideration.

-- 
Regards,
Pavel Roskin



  reply	other threads:[~2009-06-23 22:10 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-21 11:33 [PATCH] File access library for lua Bean
2009-06-22  0:42 ` Pavel Roskin
2009-06-22  3:38   ` Bean
2009-06-22  3:45     ` Pavel Roskin
2009-06-22  9:44     ` Robert Millan
2009-06-22 10:26       ` Bean
2009-06-22 10:59         ` Robert Millan
2009-06-22 17:37           ` Bean
2009-06-22 18:31             ` Pavel Roskin
2009-06-22 18:49               ` Bean
2009-06-22 18:59                 ` Pavel Roskin
2009-06-22 19:15                   ` Bean
2009-06-22 19:28                     ` Pavel Roskin
2009-06-22 19:50                       ` Bean
2009-06-22 20:06                         ` Pavel Roskin
2009-06-23  9:27                           ` Bean
2009-06-23 22:10                             ` Pavel Roskin [this message]
2009-06-23 22:50                               ` Robert Millan
2009-06-26 23:25                                 ` Pavel Roskin
2009-06-24  4:41                               ` Bean
2009-06-24  5:58                                 ` Bean
2009-06-27  0:22                                 ` Pavel Roskin
2009-06-27  3:53                                   ` Bean
2009-06-27  4:04                                     ` Pavel Roskin
2009-06-27  4:14                                       ` Bean
2009-07-05  9:59                                         ` Bean
2009-07-05 13:11                                           ` Duboucher Thomas
2009-07-05 14:28                                             ` Bean
2009-06-25 22:39                           ` Vladimir 'phcoder' Serbinenko
2009-06-26 23:45                             ` Pavel Roskin
2009-07-31  8:07             ` Marco Gerards

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1245795036.3204.26.camel@mj \
    --to=proski@gnu.org \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.