Re: [Xen-devel] [PATCH v2 2/2] arm: rename tiny64.conf to tiny64_defconfig

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 20/05/2019 14:41, Volodymyr Babchuk wrote:
> Julien Grall writes:
>
> > Hi,
> >
> > First of all, please add a cover letter when you send a series. This
> > help for threading and also a place to commend on general feedback.
> Oh, okay. That was quite simple change and I didn't wanted to spam with
> extra emails. I will include cover letter next time.
>
> > Furthermore, please use scripts/{add, get}_maintainers.pl to find the
> > correct maintainers. While I agree that CCing REST is a good idea, you
> > haven't CCed all of them.
> Problem is that I used this script:
>
> $ ./scripts/get_maintainer.pl -f 
> defconfig_v2/v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch
-f is to be used on actual file in the source tree. So the result below makes 
sense. For actual patch, you have to drop the -f.

> Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Jan Beulich <jbeulich@xxxxxxxx>
> Julien Grall <julien.grall@xxxxxxx>
> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Tim Deegan <tim@xxxxxxx>
> Wei Liu <wei.liu2@xxxxxxxxxx>
> xen-devel@xxxxxxxxxxxxxxxxxxxx
>
> I was quite surprised by result myself. Honestly, I wanted to CC only
> you and Stefano, but decided to play by the rules.
>
>
> Also, add_maintainers.pl just ignores this patch at all:
>
> % scripts/add_maintainers.pl -v 2 -d defconfig_v2
> Processing: v2-0001-makefile-add-support-for-_defconfig-targets.patch
> Processing: v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch
> ./scripts/get_maintainer.pl: file 
> 'defconfig_v2/v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch' doesn't 
> appear to be a patch.  Add -f to options?
I have just tried it and can't find the same error. Could you provide more 
details? Such as where to do call from the exact content of each patches...
> > On 16/05/2019 14:37, Volodymyr Babchuk wrote:
> > > As build system now supports *_defconfig rules it is good to be able
> > > to configure minimal XEN image with
> > I am afraid this is not correct. tiny64 will not be able to generate a
> > minimal config to boot on any platform supported by Xen.
> >
> > It is meant to be used as a base for tailoring your platform where all
> > the options are turned off by default.
> >
> > So I think offering a direct access is likely going to be misused in
> > most of the cases without proper documentation.
> In the original commit message Stefano suggested to use olddefconfig:
>
> "   Add a tiny kconfig configuration. Enabled only the credit scheduler.
>      It only carries non-default options (use make menuconfig or make
>      olddefconfig to produce a complete .config file). "
>
> I don't see any significant difference between
Did you actually try the two approach and see how they differ?

> # cp tiny64.conf .config && make olddefconfig
This one will ask you details on the configuration you want while...

> and
>
> # make tiny64_defconfig
... this one will hide the questions.

> Anyways, it is up to you to accept or decline this particular patch. I
> mostly interested in the first patch in the series, because our build
> system depends on it. This very patch I sent out only because I wanted
> to tidy up things a bit. But if you are saying that it is intended to
> store minimal config in this way, I'm okay with it.
The point of review is to discuss on the approach and find a common agreement.

If you read my previous e-mail, I didn't completely reject the approach in my 
previous e-mail. I pointed out that the user may be misled of the name and hence 
documentation would be useful.
But


Cheers,

--
Julien Grall

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