[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