[Dextrose] [PATCH] 1-to-N Feature.

Ajay Garg ajay at activitycentral.com
Wed May 2 02:02:17 EDT 2012


Sir,
Regards.



On Mon, Apr 30, 2012 at 7:38 PM, Sascha Silbe <silbe at activitycentral.com>wrote:

> Excerpts from Ajay Garg's message of 2012-04-26 23:53:51 +0200:
>
> > The feature specs, and workflow-screenshots, are listed at ::
> > http://wiki.sugarlabs.org/go/Features/Transfer_to_many_options
>
> This patch is HUGE. Please split it into several patches,


The patch will be split, once the files at "src/webdav/*" and
"src/webdav/acp/*" will be packaged into a rpm.

Actually, I had to made two changes in src/webdav/WebdavResponse.py, so
that the "GetLastModified()" and "GetCreationDate()" return the values in
timestamp format (in conformance with what is required in listview).
Perhaps, we should then make a (separate) git repo of it.

Let me sleep over this for a while :)







> based on
> functional changes. The cover letter should explain the architecture of
> your solution (e.g. using a third-party WebDAV client library rather
> than glib / gio) and why you chose it.
>


Done.






>
>
> [...]
> >  create mode 100644 src/webdav/Condition.py
> [...]
>
> This looks like an embedded copy of some external component. Never do
> that. Even if we had the resources to maintain the embedded copy
> (updating it whenever upstream publishes a new version), its history
> would be needlessly mingled with that of Sugar and we'd need to release
> the combination when either of the parts gets updated. The current
> structure of Sugar is enough of a mess already, we don't need to add to
> that.
>
> If the library isn't packaged for most of the major distros, we should
> consider whether it provides significant advantage over the alternatives
> and if - after that consideration - we still want to use it, try to get
> it into distros. If that fails, we can build a custom package for
> Dextrose. Bundling it with Sugar, however, is not a useful option.
>


Yep, same feeling here.
So, a rpm will be formed.




>
> [configure.ac]
> > @@ -61,8 +61,6 @@ extensions/cpsection/modemconfiguration/config.py
> >  extensions/cpsection/Makefile
> >  extensions/cpsection/network/Makefile
> >  extensions/cpsection/power/Makefile
> > -extensions/cpsection/updater/backends/Makefile
> > -extensions/cpsection/updater/Makefile
>
> What does WebDAV support have to do with the software update CP section?
>

Well, I had removed this, since sugar-jhbuild wasn't compiling otherwise.
Tested on an image with this change, "Software Update" worked fine.





>
>
> [src/jarabe/journal/journalactivity.py]
> > @@ -485,6 +485,9 @@ class JournalActivity(JournalWindow):
> >      def is_editing_mode_present(self):
> >          return self._editing_mode
> >
> > +    def get_volumes_toolbar(self):
> > +        return self._volumes_toolbar
>
> Giving some other component access to an internal part of the Journal
> Activity is usually going to be the wrong approach. Consider adding
> appropriate API to do only what you need. Depending on what that is,
> possible approaches may be:
>
> 1. Add API to the other component and let the Journal Activity or the
>   Volumes Toolbar take action based on that information
>   (e.g. deactivate some buttons for read-only mounts).
>
> 2. Add a signal source to the other component and let the Journal
>   Activity or the Volumes Toolbar subscribe ("connect") to it. Useful
>   if the information based on which the Journal Activity needs to
>   base its decision can change at any time.
>
> 3. Add API to the Journal Activity to pass through information from
>   the Volumes Toolbar so that the other component can query it. It's
>   worth considering why you are asking the Journal Activity,
>   though. It's the top-level component and should connect the pieces,
>   not usually be queried by one of the pieces.
>
> 4. Add a signal source to the Journal Activity to broadcast
>   information needed by the other component. Similar to 3., but can
>   be used for information that can change at any time.
>


Well, I am reluctant to do this, primarily because the signal-mechanism is
very messy (as far as code-browsing is concerned).
I remember I had to spend quite too much time for finding what ultimately
happens with the 'volume-error' signal (think there were about 5 levels of
re-direction).

Moreover, I think that that 'signals' are more suited in situations when
the 'cause' component performs some action asynchronously (which is not the
case here; we just need to perform post-actions synchronously, as and when
the 'cause' action happens).

Another thing, that 'JournalActivity' and its 'volumestoolbar' constiturent
are on a singleton basis; so we do not need to worry about mangling
different instances :)



>
> [src/jarabe/journal/journaltoolbox.py]
> > @@ -624,6 +624,13 @@ class BatchEraseButton(ToolButton,
> palettes.ActionItem):
> >                                       show_not_completed_ops_info=True)
> >          self.props.tooltip = _('Erase')
> >
> > +        # De-sensitize Batch-Erase button, for
> locally-mounted-remote-shares.
> > +        from jarabe.journal.journalactivity import get_mount_point
> > +        current_mount_point = get_mount_point()
> > +
> > +        if
> model.is_mount_point_for_locally_mounted_remote_share(current_mount_point):
> > +            self.set_sensitive(False)
>
> Similar issue here. Connecting to a signal that broadcasts the change of
> the active mount point may be a good choice, but make sure to put it at
> a reasonable level, not in the individual button.
>
>
> I'll stop here for now; will probably wait for the next revision before
> commenting on the rest of the changes.
>
> Sascha
>
> --
> http://sascha.silbe.org/
> http://www.infra-silbe.de/
>



Regards,
Ajay
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sugarlabs.org/archive/dextrose/attachments/20120502/07fe8e2e/attachment.html>


More information about the Dextrose mailing list