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

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to