<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi,<br>
<br>
I am working on my code, again.<br>
i have understood I should provide clean patch, each one with a
specific functionality.<br>
Right now, my commit are quite a mess, but I think I have learn
git enough to clean that.<br>
Would you accept my patches if i provide :<br>
<ol>
<li>A patch reorganizing the media in the HTML gallery
(actually, everything is in the root folder and you can't find
easily the index.html file)</li>
<li>A patch allowing to activate or deactivate the generation of
the files used in the gallery (the index files and the html
image files)</li>
<li>A patch allowing to create a JS database when exporting an
HTML gallery</li>
<li>A patch with my gallery using the JS database ?</li>
</ol>
<p>i will also correct the points Johannes gives me to correct as
much as I can.<br>
</p>
Cheers,<br>
<br>
Samuel Kay<br>
<br>
<br>
PS : Live demo is here (not the very last version) : <a
class="moz-txt-link-freetext"
href="http://poivron-robotique.fr/Demo_KPA_export/"><a class="moz-txt-link-freetext" href="http://poivron-robotique.fr/Demo_KPA_export/">http://poivron-robotique.fr/Demo_KPA_export/</a></a><br>
<br>
<br>
Samuel Kay wrote:<br>
</div>
<blockquote cite="mid:5AA1A3D2.4080804@401kay.fr" type="cite">Hi,
<br>
<br>
Here are my patches.
<br>
Sadly, I haven't find a way to correct :
<br>
* 1b. Since my code was written, I what to do to correct that
<br>
* 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 ?
<br>
* 3b. I find some occurrences but not a lot. I hope I got all
of them.
<br>
<br>
I manage 2 and 5 quite easily.
<br>
<br>
Cheers,
<br>
<br>
Samuel
<br>
<br>
<br>
Samuel Kay a écrit :
<br>
<blockquote type="cite">Hi Johannes
<br>
<br>
I will try to correct most of the issues below.
<br>
but :
<br>
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
<br>
Otherwise, I think I will be able to provide you some cleaner
patchs.
<br>
<br>
Thank you for having review my code.
<br>
<br>
Cheers,
<br>
<br>
Samuel
<br>
<br>
<br>
Johannes Zarl-Zierl a écrit :
<br>
<blockquote type="cite">Hi Samuel,
<br>
<br>
I did a first proper sweep over your patches. Apologies in
advance for being
<br>
picky ;-)
<br>
<br>
Here are some hints for improvement:
<br>
<br>
(1) Patch format
<br>
These issues are more a formality, but make my life as
reviewer a little
<br>
easier:
<br>
1a. Please use spaces for indentation.
<br>
<br>
1b. Group patches in a semantic way.
<br>
E.g.:
<br>
Patch 1: Add theme files for js_export
<br>
Patch 2: Implement js database export
<br>
etc.
<br>
<br>
<br>
(2) Licenses
<br>
Since you include a third-party JavaScript library, you should
also add their
<br>
license file to the theme (and copy it when exporting).
<br>
<br>
<br>
(3) Coding conventions
<br>
3a. Please declare variables "as late as possible" (usually on
first use).
<br>
This improves readability.
<br>
seen here:
<br>
[Patch 2 -> "QString Images_data, Relations, Categories;"]
<br>
<br>
3b. Prefer "if (!condition)" over "if (condition == false)"
<br>
<br>
<br>
(4) TODOs
<br>
You introduced a few TODOs/question comments in the new code.
I didn't have
<br>
time yet to consider them in context. I'll comment on those
when I've had the
<br>
time…
<br>
<br>
<br>
(5) boolean values in kphotoalbum.theme file
<br>
There's already a boolean option "Default". For consistency's
sake, please use
<br>
"true" and "false" for the new keys as well.
<br>
<br>
<br>
So far, that's all I could find.
<br>
<br>
Cheers and thanks!
<br>
Johannes
<br>
<br>
<br>
<br>
<br>
<br>
<br>
On Sonntag, 28. Jänner 2018 14:09:35 CET Samuel Kay wrote:
<br>
<blockquote type="cite">Hi,
<br>
<br>
Once more, here is the patch, after having done "git
rebase", an after
<br>
having corrected two issues :* Special char were not handle
correctly* Only
<br>
the categories selected by the user are displayed.So, the
export should be
<br>
working OK.
<br>
<br>
In KPhotoalbum, go to :* *File* => *HTML Export*.In the
*Layout* tab, select
<br>
the "*Dynamic JS*" theme.
<br>
<br>
For now, in the "Image Sizes" field, only the highest
resolution will be
<br>
used, but all selected resolution will generated.
<br>
<br>
Things that I may do :* I still want to put the photos in
separated folder*
<br>
Use the description an the title set by the user* Manage
Video
<br>
<br>
Cheers,
<br>
<br>
Samuel
<br>
<br>
<br>
Samuel Kay a écrit :
<br>
<br>
<br>
Hi,
<br>
<br>
I have a working version of the JS export. There are a few
things I want to
<br>
improve : * Actually, KPhotoAlbum change the file extension
of the resize
<br>
image (ie: JPG => jpg). I don't understand why. But with
last patch, the JS
<br>
export is using the name given by KPhotoAlbum (not the
original file name).
<br>
* I don't use the categories that are selected by the user
and I display
<br>
every category in the search panel. * The output give a lot
of file in one
<br>
folder. User should search the index file. I think it would
be nice to put
<br>
the photos in separate folder (depending of their size) and
other generate
<br>
html file in another folder.
<br>
<br>
But right now, here are my patch!
<br>
<br>
Cheers,
<br>
<br>
Samuel
<br>
</blockquote>
_______________________________________________
<br>
KPhotoAlbum mailing list
<br>
<a class="moz-txt-link-abbreviated" href="mailto:KPhotoAlbum@mail.kdab.com">KPhotoAlbum@mail.kdab.com</a>
<br>
<a class="moz-txt-link-freetext" href="https://mail.kdab.com/mailman/listinfo/kphotoalbum">https://mail.kdab.com/mailman/listinfo/kphotoalbum</a>
<br>
</blockquote>
_______________________________________________
<br>
KPhotoAlbum mailing list
<br>
<a class="moz-txt-link-abbreviated" href="mailto:KPhotoAlbum@mail.kdab.com">KPhotoAlbum@mail.kdab.com</a>
<br>
<a class="moz-txt-link-freetext" href="https://mail.kdab.com/mailman/listinfo/kphotoalbum">https://mail.kdab.com/mailman/listinfo/kphotoalbum</a>
<br>
</blockquote>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
KPhotoAlbum mailing list
<a class="moz-txt-link-abbreviated" href="mailto:KPhotoAlbum@mail.kdab.com">KPhotoAlbum@mail.kdab.com</a>
<a class="moz-txt-link-freetext" href="https://mail.kdab.com/mailman/listinfo/kphotoalbum">https://mail.kdab.com/mailman/listinfo/kphotoalbum</a>
</pre>
</blockquote>
<br>
</body>
</html>