[sugar] [PATCH] Bundlebuilder use manifest, fix_manifest function

[Jameson "Chema" Quinn]

Here is the revised patch. It has your suggested changes, plus a couple
more:

- Check for existence of po directory in Builder
- Config.__init__() is cleaned up. Now gets bundle name from activity.info.
start() no longer needs a bundle name, and has deprecation warning. Also I
put things in a more logical order.

On Sun, Jun 8, 2008 at 7:11 AM, Marco Pesenti Gritti <mpgritti at gmail.com>
wrote:

> On Sun, Jun 8, 2008 at 1:59 PM, Jameson Chema Quinn
> <jquinn at cs.oberlin.edu> wrote:
> >> It doesn't make sense for activitybundle to expose something a generic
> >> as list_files, especially since you are just using it to walk files
> >> inside that module.
> >
> > So I should duplicate the code of list_files? (This is my biggest
> question
> > with your response)
>
> Unless I'm missing something you won't really duplicate much of it,
> since you are not using the ignore_* which are the big part of the
> code and the actual purpose of that function. You will basically just
> need the for...
>
> >>
> >> I think the MANIFEST definition is overkill and you are using it
> >> inconsistently anyway. Just use the string.
> >
> > As the number of metadata files in the activity format grows, it seems
> > likely that eventually we'll give them their own directory. Making this a
> > global now is the first step to being able to change it later. But if you
> > prefer, I'll use a string for now.
>
> If you move it to a directory you will better construct to the path
> with os.path.join, so it would probably be a method of Bundle. Anyway
> I don't think it's worth to worry about that yet, it's easy enough to
> refactor if/when necessary.
>
> >>
> >>
> >> +    def __init__(self, bundle_name, source_dir=None, dist_dir = None,
> >> +                 dist_name = None):
> >>
> >> As I explained in my previous mail, I want to keep passing in the
> >> config here. Please remove the extra params.
> >
> > disagree a bit, but will do.
>
> Just look at all the ugly None checks you need to do to support it... :)
>
> >> +    def install(self):
> >>
> >> I don't think installation should be expose by ActivityBundle, we need
> >> to cleanup the dependencies. Let's remove it from this patch and we
> >> can discuss how to do it properly
> >
> > install was already exposed, I just refactored unpack out of it (and thus
> > also exposed unpack). This only showed up in the patch because I put
> unpack
> > first, and the body of unpack matched with that part of the body of
> install.
> > I'm pretty sure this function is used by Journal, we can't just drop it.
>
> You are right, That's ok then.
>
> Thanks,
> Marco
> _______________________________________________
> Sugar mailing list
> Sugar at lists.laptop.org
> http://lists.laptop.org/listinfo/sugar
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.laptop.org/pipermail/sugar/attachments/20080608/3b2abc81/attachment-0001.htm 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bundleMANIFEST.formarco.patch
Type: text/x-diff
Size: 19134 bytes
Desc: not available
Url : http://lists.laptop.org/pipermail/sugar/attachments/20080608/3b2abc81/attachment-0001.patch