Hello Alex,
Thank you for these updates, the code looks really solid !
Here are a couple of suggestions I have for these change-sets:
For the commit:
bitbake: runqueue, build, dsi: event data
change
+ task.save()
+
+ if isinstance(event, bb.build.TaskBase):
+ task.recipe.name = event._package
+ task.recipe.save()
The first save() can go away, as there is another task.save() call
a few lines down.
-----
Would it be better to change this:
self._message = "recipe %s: task %s: %s" % (d.getVar("PF",
True), t, self.getDisplayName())
into this:
self._message = "recipe %s: task %s: %s" % (self._package, t,
self.getDisplayName()) ?
------
identifier = task_information['recipe'].file_path +
task_information['task_name']
This may be a very long key. Maybe we can do something like this:
identifier =
task_information['recipe'].file_path.split('/')[-1] +
task_information['task_name']
This will store only the recipe name+version together with the
task_name
If this point is taken, this line:
identifier = event.taskfile + event.taskname
Should become:
identifier = event.taskfile.split('/')[-1] + event.taskname
! Beware of other lines that may need to change together with
this.
=====================================================
For commit:
bitbake: dsi: refactor the BuildInfoHelper
code
- return machine_info
+ pass
def create_machine_object(self, machine_information):
Do we need this 'pass' here ?
---
+ task_object.outcome=task_information['outcome']
+
+
task_object.task_executed=task_information['task_executed']
Can we delete the space between these lines ? It makes the first
line stand out from the rest and there's no reason for that.
---
+ import traceback
It's best to have all imports at the beginning of the file.
-----
Get task object and get recipe object are not consistent with
eachother.
get_recipe should consider the created attribute before trying to
store information:
+ try:
+ recipe_object.name=recipe_information['name']
+ recipe_object.version=recipe_information['version']
+ recipe_object.summary=recipe_information['summary']
+
recipe_object.description=recipe_information['description']
+ recipe_object.section=recipe_information['section']
+ recipe_object.license=recipe_information['license']
+
recipe_object.licensing_info=recipe_information['licensing_info']
+ recipe_object.homepage=recipe_information['homepage']
+
recipe_object.bugtracker=recipe_information['bugtracker']
+ recipe_object.author=recipe_information['author']
+ except:
+ pass
+ finally:
+ recipe_object.save()
And also, if we put this in a try except block, maybe it is best
to print the traceback then let it go.
We might consider to use a try/except block to the get_task
method.
---------
+ for bl in sorted(self.internal_state['build_layers'],
reverse=True, key=_slkey):
+ if (path.startswith(bl.layer.local_path)):
+ return bl
Should we add a comment here with what's the motivation behind
this ?
--------------
+ identifier = event.taskfile + event.taskname
This also needs to change if we consider my advice on using
smaller identifier values, from above.
--------------
+ if isinstance(event, bb.runqueue.runQueueTaskCompleted):
+ task_information['outcome'] = 3 # TODO: needs to
use constants
The Django model updates from this patch states the default for
this is value 4.
Either this or that need to change for consistency reasons.
--------------
+ e = event
+ e.taskname = pn
Do we need to do this? I think event instead of e makes the code
more clear.
------------
+ task_obj =
self.orm_wrapper.update_task_object(task_info)
Caution: This method doesn't return anything, so it may be better
just to call it, instead of assigning it to an attribute:
self.orm_wrapper.update_task_object(task_info)
-------------
That's about it :)
I hope this helps!
Thank you,
Calin
On 19.07.2013 01:35, Damian, Alexandru wrote: