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

[Samuel Kay]

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