[sugar] Re: [PATCH] Bundle API now provides write functionality.

[Dan Williams]

Marco,

Andrew commented yesterday that the use-case is so that Develop can
write the activity's .info file without having to duplicate code.

The reason I like this patch is that it doesn't duplicate code.

The reason I don't like the "set*" parts of this patch is that it
provides API to let any activity modify it's own .info file at any time,
which should be _clearly_ forbidden.  If you give people API, they will
use it.

So I don't know.  On the balance, I feel like .info files should be
read-only and those activities that need to modify one should do it
themselves.  We've got a .info spec anyway, so it should be relatively
stable.

Dan

On Fri, 2007-03-02 at 12:21 +0100, Marco Pesenti Gritti wrote:
> On Wed, 2007-02-28 at 23:23 -0500, Andrew Clunis wrote:
> > * now provides property setters and write_info() method, which will write
> >   the activity metadata back to activity.info.
> > * now supports the comment and host_version fields.
> > * various cleanups.
> > ---
> >  sugar/activity/bundle.py |  128 ++++++++++++++++++++++++++++++++++++++--------
> >  1 files changed, 105 insertions(+), 23 deletions(-)
> > 
> > diff --git a/sugar/activity/bundle.py b/sugar/activity/bundle.py
> > index 6a6ebdd..482b105 100644
> > --- a/sugar/activity/bundle.py
> > +++ b/sugar/activity/bundle.py
> > @@ -8,18 +8,25 @@ _PYTHON_FACTORY='sugar-activity-factory'
> >  
> >  class Bundle:
> >      """Info about an activity bundle. Wraps the activity.info file."""
> > +    
> > +    section = 'Activity'
> > +    
> 
> Please move this up to the module and call it _SECTION.
> 
> >+        if cp.has_option(self.section, 'host_version'):
> >+            self._host_version = int(cp.get(self.section,
> >'host_version'))
> 
> This is actually a required field. Though I'm not quite sure the spec
> about this is finalized so it's not worth breaking activities. Just put
> a "FIXME this should be a required field" or something.
> 
> >      def get_icon(self):
> > +        """Get the activity icon file name"""
> > +        activity_path = os.path.join(self._path, 'activity')
> > +        icon = os.path.join(activity_path, self._icon_name + ".svg")
> > +        return icon
> > +    
> 
> Use get_icon_path and """Get the activity icon path"""
> 
> > +    def get_icon_name(self):
> >          """Get the activity icon name"""
> > -        return self._icon
> > +        return self._icon_name
> > +    
> > +    def set_icon_name(self, icon_name):
> > +        """Set the activity icon name"""
> > +        self._icon_name = icon_name
> >  
> >      def get_activity_version(self):
> >          """Get the activity version"""
> >          return self._activity_version
> > +    
> > +    def set_activity_version(self, activity_version):
> > +        """Set the activity version"""
> > +        self._activity_version = activity_version
> > +        
> > +    def get_host_version(self):
> > +        """Get the host version required for this activity"""
> > +        return self._host_version
> > +    
> > +    def set_host_version(self, host_version):
> > +        """Set the host version required for this activity"""
> > +        self._host_version = host_version
> >  
> >      def get_exec(self):
> >          """Get the command to execute to launch the activity factory"""
> >          return self._exec
> > +    
> > +    def set_exec(self, exec_name):
> > +        """Set the command to execute to launch the activity factory"""
> > +        self._exec = exec_name
> >  
> >      def get_class(self):
> >          """Get the main Activity class"""
> > -        return self._exec
> > +        return self._class
> > +    
> > +    def set_class(self, klass):
> > +        """Set the main Activity class"""
> > +        self._class = klass
> >  
> >      def get_show_launcher(self):
> >          """Get whether there should be a visible launcher for the activity"""
> >          return self._show_launcher
> > +    
> > +    def set_show_launcher(self, show_launcher):
> > +        self._show_launcher = show_launcher
> > +
> > +    def write_info(self):
> > +        """Write changed configuration parameters back to the activity.info
> > +        file"""
> > +        cp = ConfigParser()
> > +        info_fd = open(self._info_path, "wb")
> > +        cp.add_section(self.section)
> > +        cp.set(self.section, "name", self._name)
> > +        cp.set(self.section, "icon", self._icon_name)
> > +        cp.set(self.section, "service_name"er, self._service_name)
> > +        if self._comment is not None:
> > +            cp.set(self.section, "comment", self._comment)
> 
> There is no "comment" in the spec.
> 
> http://wiki.laptop.org/go/Activity_Bundles
> 
> What's the use case?
> 
> Marco
> 
> _______________________________________________
> Sugar mailing list
> Sugar at laptop.org
> http://mailman.laptop.org/mailman/listinfo/sugar