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

[Sascha Silbe]

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, 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.


[...]
>  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.


[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?


[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.


[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/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.sugarlabs.org/archive/dextrose/attachments/20120430/2eca5073/attachment.pgp>