xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Dario Faggioli <dario.faggioli@citrix.com>
To: George Dunlap <dunlapg@umich.edu>, Jan Beulich <JBeulich@suse.com>
Cc: Wei Liu <wei.liu2@citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v2 3/3] xl: new "loglvl" command
Date: Tue, 8 Mar 2016 19:05:06 +0100	[thread overview]
Message-ID: <1457460306.3102.277.camel@citrix.com> (raw)
In-Reply-To: <CAFLBxZY+DhV3gXTwmoE79hE4n+jmBkTjtY49_BPm4OOHkNdsCw@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 2194 bytes --]

On Tue, 2016-03-08 at 14:05 +0000, George Dunlap wrote:
> On Tue, Mar 8, 2016 at 8:08 AM, Jan Beulich <JBeulich@suse.com>
> wrote:
> > 
> > Right, and asking people myself to not follow bad examples when
> > adding new code, I did take all of your input to adjust the patch.
> > Just that in this case the set of bad examples is so large that in
> > a
> > similar case in the hypervisor I probably wouldn't have dared to
> > ask for a style correction.
> Well the approach of the libxl maintainers seems to have be, "Just
> make sure the new code adheres to the new style, and eventyally
> everything will be up-to-date".
>
Funnily enough, basing on my experience, libxl does not look that bad
to me, and every time I've been bitten by something like this, it was
in Xen rather than in libxl. :-D

Of course, although I've been active in both, I don't claim that my
experience is statistically significant... I guess it depends on what
specific areas of code one gets to work on.

Anyway, I personally don't think this affect in any way the point that
new code should comply as much as possible with coding style, existing
best practises, etc., and that is true for this patch, as well as for
all the times everyone of us may have been asked to do the same, either
in xen, tools, or anywhere...

In fact, especially if we decide to do this (which I'd be in favour of,
and up for helping):

> Given that the "new" style has been around for a while now, it
> probably would be good to set aside some time at the beginning of the
> next development cycle to fix things up
>
being strict about new code actually helps this, as it makes sure there
is less --rather than more-- code to fix during such a huge fixup
challenge! :-)

>  -- it is incredibly
> frustrating to carefully try to copy the surrounding style, only to
> be
> told to revise it.
> 
Yep, I fully agree.

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 #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

  parent reply	other threads:[~2016-03-08 18:05 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-04 16:38 [PATCH v2 0/3] allow runtime log level threshold adjustments Jan Beulich
2016-03-04 16:46 ` [PATCH v2 1/3] console: allow " Jan Beulich
2016-03-04 20:55   ` Konrad Rzeszutek Wilk
2016-03-07 10:44     ` Jan Beulich
2016-03-07 14:41       ` Konrad Rzeszutek Wilk
2016-03-07 15:19         ` Jan Beulich
2016-03-04 16:47 ` [PATCH v2 2/3] libxc: wrapper for log level sysctl Jan Beulich
2016-03-05 16:00   ` Dario Faggioli
2016-03-08 16:20   ` Wei Liu
2016-03-04 16:48 ` [PATCH v2 3/3] xl: new "loglvl" command Jan Beulich
2016-03-04 18:45   ` Dario Faggioli
2016-03-07 11:46     ` Jan Beulich
2016-03-07 18:07       ` Dario Faggioli
2016-03-08  8:08         ` Jan Beulich
2016-03-08 14:05           ` George Dunlap
2016-03-08 16:09             ` Wei Liu
2016-03-08 18:05             ` Dario Faggioli [this message]
2016-03-05 15:36   ` Dario Faggioli
2016-03-07 13:20   ` Fabio Fantoni
2016-03-07 13:26     ` Jan Beulich
2016-03-08 16:20   ` Wei Liu
2016-03-14 15:23     ` Jan Beulich
2016-03-14 15:36       ` Wei Liu
2016-03-14 15:49         ` Jan Beulich
2016-03-14 16:01           ` Wei Liu
2016-03-14 17:00             ` Jan Beulich
2016-03-14 17:07               ` Ian Jackson
2016-03-15  7:37                 ` Jan Beulich
2016-03-15 13:58                   ` Wei Liu
2016-03-15 14:07                     ` Jan Beulich
2016-03-15 14:51                       ` Wei Liu
2016-03-15 15:03                         ` Jan Beulich
2016-03-15 15:38                       ` Ian Jackson
2016-03-16 11:22                         ` Jan Beulich
2016-04-28 15:33                           ` Wei Liu
2016-04-29  7:20                             ` Jan Beulich
2016-05-02 11:14                               ` Wei Liu

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=1457460306.3102.277.camel@citrix.com \
    --to=dario.faggioli@citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=JBeulich@suse.com \
    --cc=dunlapg@umich.edu \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).