[Bugs] #1719 UNSP: resume journal entry race may duplicate resumed activity id
Sugar Labs Bugs
bugtracker-noreply at sugarlabs.org
Wed May 19 06:57:43 EDT 2010
#1719: resume journal entry race may duplicate resumed activity id
------------------------------------------+---------------------------------
Reporter: quozl | Owner: alsroot
Type: defect | Status: reopened
Priority: Unspecified by Maintainer | Milestone: 0.88.x
Component: sugar | Version: Git as of bugdate
Severity: Major | Resolution:
Keywords: r! | Distribution: Unspecified
Status_field: Assigned |
------------------------------------------+---------------------------------
Changes (by tomeu):
* keywords: r? => r!
Comment:
This patch is very different from what I reviewed previously, would be
helpful if you could write a short comment explaining why.
{{{
if home_activity.props.launch_status == home_activity.LAUNCHING:
}}}
Constants truly belong to the classes or modules they are defined in, not
to instances, so Activity.LAUNCHING.
{{{
self._launch = self.LAUNCHING
}}}
launch is a verb, what's wrong with self._launch_status like in other
parts of this patch?
{{{
def push_launcher(self, activity_id, launcher):
...
def pop_launcher(self, activity_id):
}}}
I find a bit confusing that we use push/pop for something that is not
really a stack.
{{{
def _on_failed_launch(self):
# TODO
pass
}}}
I think this deserves a ticket (referenced from the comment) and an email
to the design team if you have time. I guess a badge would be useful here.
Otherwise the patch looks good, but as I have had to fix several bugs in
this same code before, it would have helped if you had explained why you
need to do the refactoring you did.
--
Ticket URL: <http://bugs.sugarlabs.org/ticket/1719#comment:10>
Sugar Labs <http://sugarlabs.org/>
Sugar Labs bug tracking system
More information about the Bugs
mailing list