[Toaster] [review-request] adamian/20150707_bugs

[Damian, Alexandru]

Hi,

The patch that Michael suggested to fix is now refactored, and the patchset
rebased.

I pushed the changes on the same branch (force update).

Can you please review ?

Cheers,
Alex

On Mon, Jul 13, 2015 at 5:30 PM, Michael Wood <michael.g.wood at intel.com>
wrote:

>
> Could you include the summary of each of the commits for review so that we
> can be sure which patches are the new ones
>
> * 3b0073f bitbake: toaster: fix updates on failed build requests
> * 1bdd687 bitbake: toaster: replace raising Exceptions in loadconf
> * 99a626a bitbake: toaster: do not stop data import on bad data
>
> Look fine to me
>
> * 4fc46e7 bitbake: toastergui: fixing pylint warnings
>
> This one needs splitting up, pylint fixes, logging fixes , string to int
> casting, etc
>
> Thanks,
>
> Michael
>
>
>
> On 08/07/15 15:52, Barros Pena, Belen wrote:
>
>>
>> On 07/07/2015 17:27, "Damian, Alexandru" <alexandru.damian at intel.com>
>> wrote:
>>
>>  Hi,
>>>
>>>
>>> I have some comments below,
>>>
>>>
>>> Alex
>>>
>>> On Tue, Jul 7, 2015 at 3:54 PM, Barros Pena, Belen
>>> <belen.barros.pena at intel.com> wrote:
>>>
>>>
>>>
>>> On 07/07/2015 15:10, "Damian, Alexandru" <alexandru.damian at intel.com>
>>> wrote:
>>>
>>>  Hello,
>>>>
>>>>
>>>> I'm pushing two patches for review
>>>>
>>>>
>>>> - one is fixing 7955
>>>>
>>> Thanks for the quick fix, Alex! I've tested this on master, and I've
>>> noticed a couple of things:
>>>
>>> 1. The invalid data from the layer index causes warnings when importing
>>> the information. This might be because the debug mode is enabled, though,
>>> but I thought I'd bringing it up just in case.
>>>
>>> ​This happens because data doesn't match what Toaster expect - Toaster
>>> has a bit stricter ​requirements than Layer Index. I would expect that
>>> the warning messages are helpful to the user, and they should not be
>>> obscured. If a particular user wishes to not see some of the messages,
>>> they can set up the debug level to something higher in settings.py. Or we
>>> can ship with a higher debug level by default, but I don't think we
>>> should silently ignore bad data.
>>>
>> Sure: I can see your point. I was only bringing it up, but I am not sure
>> what's best, to be honest: showing them or not. As you said, falling
>> silent doesn't sound right; on the other hand, the messages look a bit
>> alarming.
>>
>>
>>  2. I can see at least one recipe in the 'all recipes' table without a
>>> name. This particular one is provided by meta-ivi. You can add the layer
>>> and you get a build button, which you can click, although when you do so
>>> the build does not seem to start. I think we need to hide any recipes
>>> that
>>> do not have a name from the list. They are invalid, and should not be
>>> exposed to  users.
>>>
>>> ​We can add such a check, of course, on imported data. I would say this
>>> is the object of a different bug report, though.
>>>
>> Done! https://bugzilla.yoctoproject.org/show_bug.cgi?id=7969
>>
>>  Also, I think we are going to need to back port the fix to Fido.
>>>
>> Is back porting ok? Should we track this somehow?
>>
>> Thanks!
>>
>> Belén
>>
>>  Cheers
>>>
>>> Belén
>>>
>>>  - one is fixing various issues highlighted by pylint
>>>>
>>>>
>>>>
>>>> https://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=adamian/20
>>>> 1
>>>> 50707_bugs
>>>>
>>>>
>>>>
>>>> ​Can you please review ?
>>>>
>>>>
>>>> Cheers,
>>>> Alex​
>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Alex Damian
>>>> Yocto Project
>>>>
>>>> SSG / OTC
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> --
>>> Alex Damian
>>> Yocto Project
>>>
>>> SSG / OTC
>>>
>>>
>>>
>>>
>>>
> --
> _______________________________________________
> toaster mailing list
> toaster at yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster
>



-- 
Alex Damian
Yocto Project
SSG / OTC
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.yoctoproject.org/pipermail/toaster/attachments/20150715/7ff5b6ba/attachment-0001.html>