Re: [Minios-devel] [PATCH v3 02/43] arm32: remove the arm32 specific code file

[Huang Shijie <shijie.huang@xxxxxxx>]

On Wed, Apr 18, 2018 at 10:45:13AM +0100, Julien Grall wrote:
> 
> 
> On 18/04/18 10:41, Huang Shijie wrote:
> >On Wed, Apr 18, 2018 at 10:38:15AM +0100, Julien Grall wrote:
> >>Hi,
> >>
> >>On 18/04/18 10:30, Huang Shijie wrote:
> >>>On Mon, Apr 16, 2018 at 04:38:20PM +0100, Julien Grall wrote:
> >>>Hi Julien,
> >>>>Hi Shijie,
> >>>>
> >>>>On 16/04/18 07:31, Huang Shijie wrote:
> >>>>>This patch removes the arm32 specific code file:
> >>>>>     arm32.S, hypercalls32.S, minios-arm32.lds
> >>>>
> >>>>How about the rest of the code? I see quite a few patch with "remove arm32
> >>>>code" in the commit message.
> >>>This patch just removes the specific files for arm32.
> >>>As you know that there are other places which have arm32 code, such as 
> >>>gic/timer.
> >>>
> >>>I prefer to remove the arm32 code while adding the arm64 correspond code
> >>>in the gic/timer file.  It is more clear to me.
> >>
> >>Patch should ideally do one logical things to help the reviewer
> >>understanding the patch with minimal effort. Removing arm32 code in patch
> >>called "implement arm64" does not make sense. Plus nowhere in that series
> >>you explain that decision which is not what we agreed on.
> >
> >Okay, I will use one patch to remove all the arm32 code in the next version.
> 
> Please read what I said below:
> 
> "it is your choice to split the series like that but if you want people to
> review it then you should help them to understand what you are doing. This
> means better commit messages and cover letter. "
> 
> By that I meant, I would be happy with what you did. But you need to update
> your commit messages accordingly. At this stage, that's probably going to be
> easier than reshuffling yet another time this series.
Yes, it is pain to reshuffling the whole patch set.

Thanks, I will add more commit message to this patch and other ones.

Huang Shijie

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel