Re: [yocto] [layerindex-web][PATCH v2 05/12] layerindex: Add distro to web interface and model.

2016-10-11 Thread Mark Hatle
On 10/10/16 3:20 AM, Paul Eggleton wrote:
> This looks good, just a few notes about this bit:
> 
> On Fri, 07 Oct 2016 11:57:14 Liam R. Howlett wrote:
>> +def update_distro_conf_file(path, distro):
>> +logger.debug('Updating distro %s' % path)
>> +desc = ""
>> +with open(path, 'r') as f:
>> +for line in f:
>> +if line.startswith('#@NAME:'):
>> +desc = line[7:].strip()
>> +if line.startswith('#@DESCRIPTION:'):
>> +desc = line[14:].strip()
>> +desc = re.sub(r'Distribtuion configuration for( running)*( 
>> an)*( the)*', '', desc) 
> 
> Aside from the typo, that last line isn't needed for distros.
> 
> Also, unlike machines, the readable distro name actually gets set in a proper
> variable (DISTRO_NAME) and though some distro conf files do still use the
> meta-comments, a lot don't use them at all. Technically being a variable,
> DISTRO_NAME could be set in an inc file rather than the distro conf file, and
> often it is, so we should really parse to get it rather than scraping it.
> 
> (I'm happy if we sort this out as a later improvement, I just wanted to make
> a note of it now.)

I have fixed the typo in the current mhatle/django18 layer.

> Cheers,
> Paul
> 

-- 
___
yocto mailing list
yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/yocto


Re: [yocto] [layerindex-web][PATCH v2 05/12] layerindex: Add distro to web interface and model.

2016-10-09 Thread Paul Eggleton
This looks good, just a few notes about this bit:

On Fri, 07 Oct 2016 11:57:14 Liam R. Howlett wrote:
> +def update_distro_conf_file(path, distro):
> +logger.debug('Updating distro %s' % path)
> +desc = ""
> +with open(path, 'r') as f:
> +for line in f:
> +if line.startswith('#@NAME:'):
> +desc = line[7:].strip()
> +if line.startswith('#@DESCRIPTION:'):
> +desc = line[14:].strip()
> +desc = re.sub(r'Distribtuion configuration for( running)*( 
> an)*( the)*', '', desc) 

Aside from the typo, that last line isn't needed for distros.

Also, unlike machines, the readable distro name actually gets set in a proper
variable (DISTRO_NAME) and though some distro conf files do still use the
meta-comments, a lot don't use them at all. Technically being a variable,
DISTRO_NAME could be set in an inc file rather than the distro conf file, and
often it is, so we should really parse to get it rather than scraping it.

(I'm happy if we sort this out as a later improvement, I just wanted to make
a note of it now.)

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre
-- 
___
yocto mailing list
yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/yocto