All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
Cc: "Taylor Simpson" <tsimpson@quicinc.com>,
	"riku.voipio@iki.fi" <riku.voipio@iki.fi>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"laurent@vivier.eu" <laurent@vivier.eu>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: Re: [PATCH] Add minimal Hexagon target - First in a series of patches - linux-user changes + linux-user/hexagon + skeleton of target/hexagon - Files in target/hexagon/imported are from another project and therefore do not conform to qemu coding standards
Date: Wed, 20 Nov 2019 08:49:25 +0100	[thread overview]
Message-ID: <59c88efa-999b-9edf-7e34-f283ac8c802e@linaro.org> (raw)
In-Reply-To: <CAL1e-=gmLywnyUoySxuDPS1FQRx=WiH1kYrqEJDGAAcO5vDg4A@mail.gmail.com>

On 11/20/19 3:26 AM, Aleksandar Markovic wrote:
> 
> 
> On Tuesday, November 19, 2019, Richard Henderson <richard.henderson@linaro.org
> <mailto:richard.henderson@linaro.org>> wrote:
> 
>     On 11/19/19 6:22 PM, Taylor Simpson wrote:
>     > - Laurent suggested I split the patch into two parts: linux-user and
>     target/hexagon.  If I do that, which one should contain the changes to
>     common files (e.g., configure)?  Also, note that we won't be able to build
>     until both patches are merged.  Is that OK?
> 
>     The configure parts should be a third, last, patch.
> 
>     The series is bisectable, because before the configure patch,
>     none of the hexagon code is compiled at all.
> 
> 
> I don't think this is a good advice. Yes, at first glance, that would make the
> submitter's job easier - since he could divide the whole code into parts
> practicaly arbitrarily - but the resulting series will be of suboptimal
> quality.

How's that?  He has been asked to split the linux-user stuff from the target
skeleton stuff.  Neither of these will compile independently.  You could merge
the configure bits artificially into the second of the two patches, or you
could leave it separate.

> If the submitter was forced from the outset to add segments of his
> solution so that each step actually (not only ostensibly) compiles, he would
> also be forced to organize patches in much more coherent way, organize his code
> in much more modular way, possibly improve initial organization, additionally
> making reviews much easier.

This argument would make more sense if there were more present here than a
skeleton.  The vast majority of the work is yet to be submitted, and *will*
have to compile, be coherent, etc, just as you suggest.


r~


  reply	other threads:[~2019-11-20  7:50 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-18 23:58 [PATCH] Add minimal Hexagon target - First in a series of patches - linux-user changes + linux-user/hexagon + skeleton of target/hexagon - Files in target/hexagon/imported are from another project and therefore do not conform to qemu coding standards Taylor Simpson
2019-11-19  1:31 ` no-reply
2019-11-19  8:51   ` Exclude paths from checkpatch (was: Re: [PATCH] Add minimal Hexagon target) Philippe Mathieu-Daudé
2019-11-19 13:33     ` Richard Henderson
2019-11-19 15:37     ` Paolo Bonzini
2019-11-19 16:14     ` Stefan Hajnoczi
2019-11-19  8:39 ` [PATCH] Add minimal Hexagon target - First in a series of patches - linux-user changes + linux-user/hexagon + skeleton of target/hexagon - Files in target/hexagon/imported are from another project and therefore do not conform to qemu coding standards Laurent Vivier
2019-11-19  9:03   ` Laurent Vivier
2019-11-19 14:14 ` Eric Blake
2019-11-19 15:11   ` Philippe Mathieu-Daudé
2019-11-19 15:19 ` Philippe Mathieu-Daudé
2019-11-19 17:22   ` Taylor Simpson
2019-11-19 17:32     ` Peter Maydell
2019-11-19 18:13     ` Laurent Vivier
2019-11-20  4:48       ` Taylor Simpson
2019-11-20  8:33         ` Laurent Vivier
2019-11-20  9:02           ` Richard Henderson
2019-11-20 12:58             ` Taylor Simpson
2019-11-20 14:14               ` Laurent Vivier
2019-11-20 15:19                 ` Taylor Simpson
2019-11-20 16:40               ` Alex Bennée
2019-11-20 17:09       ` Philippe Mathieu-Daudé
2019-11-19 19:36     ` Richard Henderson
2019-11-20  2:26       ` Aleksandar Markovic
2019-11-20  7:49         ` Richard Henderson [this message]
2019-11-21  6:01           ` Aleksandar Markovic
2019-11-21  8:55             ` Richard Henderson
2019-11-20  8:41         ` Laurent Vivier
2019-11-20 17:34         ` Alex Bennée
2019-11-19 19:33 ` Richard Henderson
2019-11-20  5:15   ` Taylor Simpson
2019-11-20  8:06     ` Richard Henderson
2019-11-20 12:51       ` Taylor Simpson
2019-11-20 14:43         ` Richard Henderson
2019-11-20 15:17           ` Taylor Simpson
2019-11-21  9:00             ` Richard Henderson
2019-11-21 19:20 ` Aleksandar Markovic
2019-11-21 19:52   ` Taylor Simpson
2019-11-21 20:44     ` Aleksandar Markovic
2019-11-21 23:51       ` Taylor Simpson
2019-11-22  9:33         ` Aleksandar Markovic

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=59c88efa-999b-9edf-7e34-f283ac8c802e@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=aleksandar.m.mail@gmail.com \
    --cc=laurent@vivier.eu \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=riku.voipio@iki.fi \
    --cc=tsimpson@quicinc.com \
    /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.