From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robert Hu Subject: Re: [OSSTEST Nested PATCH v11 2/7] Parsing grub which has 'submenu' primitive Date: Thu, 11 Jun 2015 17:01:41 +0800 Message-ID: <1434013301.32731.9.camel@localhost> References: <1432631304-27347-1-git-send-email-longtaox.pang@intel.com> <1432631304-27347-3-git-send-email-longtaox.pang@intel.com> <21880.15363.757545.925172@mariner.uk.xensource.com> <1433992665.32731.6.camel@localhost> <1434011852.30003.113.camel@citrix.com> Reply-To: robert.hu@intel.com Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1434011852.30003.113.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: robert.hu@intel.com, "longtao.pang" , Ian Jackson , wei.liu2@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Thu, 2015-06-11 at 09:37 +0100, Ian Campbell wrote: > On Thu, 2015-06-11 at 11:17 +0800, Robert Hu wrote: > > > > Unless I'm mistaken, the MenuEntryPath of a $submenu is never used ? > > > Not setting it would avoid (a) a need to rewrap and (b) me complaining > > > that you have open-coded the join twice. > > Actually this contribution from Ian Campbell. > > Hi Ian C., would you agree if I simply remove the 'MenuEntryPath' here? > > I left it as a debugging aid, since it shows up in Dumper($submenu) > which is convenient to sprinkle around while debuggiung. I don't mind if > it is removed or kept though. Forgive me that I'm to remove it, otherwise I don't know how to fix the "open-coded" criticism. > > Note that several patches from this series are already in osstest > production: > > b77a6a2 Changes to support '/boot' leading paths of kernel, xen, in grub > 997385f Parsing grub which has 'submenu' primitive > 155bdb3 Move the code for setting memory size into prep() > 2545fc6 Edit some APIs in TestSupport.pm for nested test > 699c911 Refactor installation of overlays > > So a incremental patch is what is needed here. Sure. > > I fixed one or two issues as I committed, e.g. : > > > Missing spaces after if and inside `){'. > > OK, to refine these. > > Worth double checking which I caught though. > > Ian. >