To: Paul Eggleton <paul.eggleton@linux.intel.com>
References: <cover.1497320961.git.liezhi.yang@windriver.com>
	<6b4222d19233693a64f354f37c79bb66a18d2277.1497320961.git.liezhi.yang@windriver.com>
	<1814038.FNi6Ofsz57@peggleto-mobl.ger.corp.intel.com>
From: Robert Yang <liezhi.yang@windriver.com>
Message-ID: <a4cd7aaa-2bdc-e753-74df-a170627e8153@windriver.com>
Date: Thu, 22 Jun 2017 18:53:28 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
	Thunderbird/45.2.0
MIME-Version: 1.0
In-Reply-To: <1814038.FNi6Ofsz57@peggleto-mobl.ger.corp.intel.com>
Cc: Konrad.Scherer@windriver.com, yocto@yoctoproject.org
Subject: Re: [PATCH V4 01/10] update.py: update actual branch for layer and bitbake
Precedence: list
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit

Hi Paul,

Thanks for the review, I've fixed all the issues you commented except
the following ones.

On 06/21/2017 05:01 PM, Paul Eggleton wrote:
> Hi Robert,

>>
>> +    # We deliberately exclude status == 'X' ("no update") here
>> +    layerquery_all = LayerItem.objects.filter(classic=False).filter(status='P')
>> +    if layerquery_all.count() == 0:
>> +        logger.info("No published layers to update")
>> +        sys.exit(1)
>
> So near as I can tell this change isn't required as part of the other
> changes here, it's just to tell you up front that no layers are published
> if you've specified layers instead of seeing an error. I don't have an
> issue with that but this change should be mentioned in the commit
> message ("additionally ..."), or split out as a separate patch.

Sorry, it should be in another patch, I added it to wrong place when rebase.
I add this patch into "update.py: update layers orderly" now. I need
layerquery_all to know all the provided collections in database, so that we
can know whether the dependencies match or not.

>
>
>> +    # For -a option to update bitbake branch
>> +    update_bitbake = False
>>      if options.layers:
>> -        layerquery = LayerItem.objects.filter(classic=False).filter(name__in=options.layers.split(','))
>> -        if layerquery.count() == 0:
>> -            logger.error('No layers matching specified query "%s"' % options.layers)
>> -            sys.exit(1)
>> +        layers = options.layers.split(',')
>> +        if 'bitbake' in layers:
>> +            update_bitbake = True
>> +            layers.remove('bitbake')
>
> So since this means 'bitbake' is a reserved name for layers we should add
> validation upon layer submission to ensure you can't create a layer with this
> name. I can take care of that if you like.

Good idea, I will do it later, or you can do it if you have time.

// Robert