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

[Julien Grall <julien.grall@arm.com>]

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@citrix.com>
> George Dunlap <George.Dunlap@eu.citrix.com>
> Ian Jackson <ian.jackson@eu.citrix.com>
> Jan Beulich <jbeulich@suse.com>
> Julien Grall <julien.grall@arm.com>
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Stefano Stabellini <sstabellini@kernel.org>
> Tim Deegan <tim@xen.org>
> Wei Liu <wei.liu2@citrix.com>
> xen-devel@lists.xenproject.org
> 
> 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel