[KPhotoAlbum] KPhotoAlbum - mailing list and export suggestion with code

[Samuel Kay]

Hi,

Here are my patches.
Sadly, I haven't find a way to correct :
*    1b. Since my code was written, I what to do to correct that
*    3a. I didn't find any occurrence. In your example, I wanted to 
declare the variable outside the loop since they will be used in the 
loop and after. Should I declare them in the loop ?
*    3b. I find some occurrences but not a lot. I hope I got all of them.

I manage 2 and 5 quite easily.

Cheers,

Samuel


Samuel Kay a écrit :
> Hi Johannes
>
> I will try to correct most of the issues below.
> but :
> 1b : I follow the instruction on the web site, I will try to group the 
> patchs but I have never that before. I may need some time to find the 
> good git commands
> Otherwise, I think I will be able to provide you some cleaner patchs.
>
> Thank you for having review my code.
>
> Cheers,
>
> Samuel
>
>
> Johannes Zarl-Zierl a écrit :
>> Hi Samuel,
>>
>> I did a first proper sweep over your patches. Apologies in advance 
>> for being
>> picky ;-)
>>
>> Here are some hints for improvement:
>>
>> (1) Patch format
>> These issues are more a formality, but make my life as reviewer a little
>> easier:
>> 1a. Please use spaces for indentation.
>>
>> 1b. Group patches in a semantic way.
>> E.g.:
>> Patch 1: Add theme files for js_export
>> Patch 2: Implement js database export
>> etc.
>>
>>
>> (2) Licenses
>> Since you include a third-party JavaScript library, you should also 
>> add their
>> license file to the theme (and copy it when exporting).
>>
>>
>> (3) Coding conventions
>> 3a. Please declare variables "as late as possible" (usually on first 
>> use).
>> This improves readability.
>> seen here:
>> [Patch 2 -> "QString Images_data, Relations, Categories;"]
>>
>> 3b. Prefer "if (!condition)" over "if (condition == false)"
>>
>>
>> (4) TODOs
>> You introduced a few TODOs/question comments in the new code. I 
>> didn't have
>> time yet to consider them in context. I'll comment on those when I've 
>> had the
>> time…
>>
>>
>> (5) boolean values in kphotoalbum.theme file
>> There's already a boolean option "Default". For consistency's sake, 
>> please use
>> "true" and "false" for the new keys as well.
>>
>>
>> So far, that's all I could find.
>>
>> Cheers and thanks!
>>    Johannes
>>
>>
>>
>>
>>
>>
>> On Sonntag, 28. Jänner 2018 14:09:35 CET Samuel Kay wrote:
>>> Hi,
>>>
>>> Once more, here is the patch, after having done "git rebase", an after
>>> having corrected two issues :* Special char were not handle 
>>> correctly* Only
>>> the categories selected by the user are displayed.So, the export 
>>> should be
>>> working OK.
>>>
>>> In KPhotoalbum, go to :* *File* => *HTML Export*.In the *Layout* 
>>> tab, select
>>> the "*Dynamic JS*" theme.
>>>
>>> For now, in the "Image Sizes" field, only the highest resolution 
>>> will be
>>> used, but all selected resolution will generated.
>>>
>>> Things that I may do :* I still want to put the photos in separated 
>>> folder*
>>> Use the description an the title set by the user* Manage Video
>>>
>>> Cheers,
>>>
>>> Samuel
>>>
>>>
>>> Samuel Kay a écrit :
>>>
>>>
>>> Hi,
>>>
>>> I have a working version of the JS export. There are a few things I 
>>> want to
>>> improve : * Actually, KPhotoAlbum change the file extension of the 
>>> resize
>>> image (ie: JPG => jpg). I don't understand why. But with last patch, 
>>> the JS
>>> export is using the name given by KPhotoAlbum (not the original file 
>>> name).
>>> * I don't use the categories that are selected by the user and I 
>>> display
>>> every category in the search panel. * The output give a lot of file 
>>> in one
>>> folder. User should search the index file. I think it would be nice 
>>> to put
>>> the photos in separate folder (depending of their size) and other 
>>> generate
>>> html file in another folder.
>>>
>>> But right now, here are my patch!
>>>
>>> Cheers,
>>>
>>> Samuel
>> _______________________________________________
>> KPhotoAlbum mailing list
>> KPhotoAlbum at mail.kdab.com
>> https://mail.kdab.com/mailman/listinfo/kphotoalbum
> _______________________________________________
> KPhotoAlbum mailing list
> KPhotoAlbum at mail.kdab.com
> https://mail.kdab.com/mailman/listinfo/kphotoalbum

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Dynamique-Web-Export-setting-things.patch
Type: text/x-patch
Size: 57999 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kphotoalbum/attachments/20180308/39846217/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Dynamique-Web-Export-Nearly-working.patch
Type: text/x-patch
Size: 7995 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kphotoalbum/attachments/20180308/39846217/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Dynamique-Web-Export-Working-oonly-with-thumbnail-of.patch
Type: text/x-patch
Size: 7369 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kphotoalbum/attachments/20180308/39846217/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Dynamique-Web-Export-Working.patch
Type: text/x-patch
Size: 3050 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kphotoalbum/attachments/20180308/39846217/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-Dynamique-Web-Export-Using-photo-names-given-by-KPho.patch
Type: text/x-patch
Size: 3430 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kphotoalbum/attachments/20180308/39846217/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-Dynamique-Web-Export-Small-upgrade.patch
Type: text/x-patch
Size: 3587 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kphotoalbum/attachments/20180308/39846217/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-Dynamique-Web-Export-Changing-tabulation-for-spaces.patch
Type: text/x-patch
Size: 28619 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kphotoalbum/attachments/20180308/39846217/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-Dynamique-Web-Export-Using-boolean-in-theme-config-a.patch
Type: text/x-patch
Size: 11387 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kphotoalbum/attachments/20180308/39846217/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0009-Dynamique-Web-Export-Add-Licence-for-Mustache.js.patch
Type: text/x-patch
Size: 2393 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kphotoalbum/attachments/20180308/39846217/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0010-Dynamique-Web-Export-Finalisation-of-the-patch.patch
Type: text/x-patch
Size: 4875 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kphotoalbum/attachments/20180308/39846217/attachment-0009.bin>