Review Request: Another approach for PackageStructure for multiple paths per entry

[Aaron Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5765/#review8505
-----------------------------------------------------------


i don't see why this patch removes the ability to have multiple paths per entry, since both approaches are useful though perhaps in different situations. the only downside i see to keeping searchPaths for entries is that we don't have an immediate use case for them, which makes it a slightly speculative feature if it isn't used here. i can imagine various situations where it would make sense, however.


/trunk/KDE/kdelibs/plasma/package.cpp
<http://svn.reviewboard.kde.org/r/5765/#comment8860>

    i don't think this will work: if it truly is fallback based, then it should be just fine to have a required item in any of the context prefixes.



/trunk/KDE/kdelibs/plasma/package.cpp
<http://svn.reviewboard.kde.org/r/5765/#comment8861>

    same as above



/trunk/KDE/kdelibs/plasma/package.cpp
<http://svn.reviewboard.kde.org/r/5765/#comment8862>

    the implicit assumption here is that fallbacks are listed first. that's probably fair, but should be noted in the apidox for PackageStructure. same situation with my patch, so not your oversight alone :)



/trunk/KDE/kdelibs/plasma/package.cpp
<http://svn.reviewboard.kde.org/r/5765/#comment8863>

    this means that the metadata file will be added to the hash for each contents prefix, which is incorrect. that should be moved out of the foreach loop


- Aaron


On 2010-11-04 14:44:46, Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5765/
> -----------------------------------------------------------
> 
> (Updated 2010-11-04 14:44:46)
> 
> 
> Review request for Plasma and Aaron Seigo.
> 
> 
> Summary
> -------
> 
> in relation to http://svn.reviewboard.kde.org/r/5763/
> i still can't really wrap my mind  on that approach, i still see it workig more like this:
> - the list of fallbacks is defined on the prefix
> - diffrent platforms would have things in different prefixes, to not have reserved words in the contents/ directory
> - another shell can have a packagerc with different prefix paths (before plasma/packageformats/%1rc a plasma-$app/packageformats/%1rc should be looked for)
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/package.cpp 1193070 
>   /trunk/KDE/kdelibs/plasma/packagestructure.h 1193070 
>   /trunk/KDE/kdelibs/plasma/packagestructure.cpp 1193070 
> 
> Diff: http://svn.reviewboard.kde.org/r/5765/diff
> 
> 
> Testing
> -------
> 
> i am not sure the hash still works :/
> 
> 
> Thanks,
> 
> Marco
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20101104/302d0888/attachment-0001.htm