[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [OSSTEST PATCH 1/4] Add nested testcase of preparing and installing L1 guest VM



On Fri, 2014-12-26 at 13:13 +0800, Robert Hu wrote:
> On Thu, 2014-12-11 at 11:06 +0000, Wei Liu wrote:

> > ?
> > 
> > >                  die unless $entry;
> > >                  $entry->{KernDom0}= $1;
> > >                  $entry->{KernVer}= $2;
> > >              }
> > > -            if (m/^\s*module\s*\/(initrd\S+)/) {
> > > +     if (m/^\s*module\s*(?:\/boot)*\/(initrd\S+)/) {
> > >                  $entry->{Initrd}= $1;
> > >              }
> > >          }
> > 
> > As I said before, this hunk should be in its own patch.
> > 
> > Just FYI, there are multiple people (Dario, you and I) touching this
> > piece of code. You might want to keep an eye on main OSSTest git tree
> > and rebase before reposting (and so do Dario and I).
>
Indeed. :-)

> > > diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
> > > @@ -55,8 +55,9 @@ BEGIN {
> > >                        target_putfilecontents_stash
> > >                 target_putfilecontents_root_stash
> > >                        target_put_guest_image target_editfile
> > > -                      target_editfile_root target_file_exists
> > > -                      target_run_apt
> > > +               target_editfile_root target_file_exists 
> > > +               target_file_exists_root
> > > +               target_run_apt
> > 
> > Please don't just change white spaces. This makes patches hard to
> > review.
> > 
> > >                        target_install_packages 
> > > target_install_packages_norec
> > >                        target_jobdir target_extract_jobdistpath_subdir
> > >                        target_extract_jobdistpath target_guest_lv_name
> > > @@ -67,7 +68,7 @@ BEGIN {
> > >                        selecthost get_hostflags get_host_property
> > >                        get_host_native_linux_console
> > >                        power_state power_cycle power_cycle_time
> > > -                      serial_fetch_logs
> > > +                      serial_fetch_logs select_ether
> > >                        propname_massage
> > >           
> > >                        get_stashed open_unique_stashfile compress_stashed
> > > @@ -109,6 +110,7 @@ BEGIN {
> > >                        iso_gen_flags_basic
> > >                        iso_copy_content_from_image
> > >                        guest_editconfig_nocd
> > > +               guest_editconfig_cd
> > 
> > Indentation. I think we mostly use space instead of hard tab. Ian?
> So what's the convention in Xen code writing? use space instead of tab?
> And how many space to substitute 1 tab? I used to use 4. 
>
Xen's coding style is described in various CODING_STYLE files, within
the Xen's tree.

For example, for the hypervisor, this one:
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=CODING_STYLE;h=95842e39c967ec0c100f9337ad4b245fbff1a53f;hb=refs/heads/staging

For libxl, this other one:
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/CODING_STYLE;h=f5b58909b34d158f4c06bb3e279f6648b1aa01ec;hb=refs/heads/staging

However, OSSTest is not Xen, so whatever Xen coding style is, it does
not really matter here. :-)

There actually is no formally defined coding style, and in fact, there
is no CODING_STYLE file in OSSTest's tree. The idea is to follow the
prevalent style of the file you're modifying.

For instance, in case of TestSupport.pm it seems that spaces are used,
and that indentation happens by means of 4 spaces.

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.