Re: KPhotoAlbum - mailing list and export suggestion with code

Samuel Kay <[email protected]> Sun, 21 Oct 2018 00:35:01 +0200

Newsgroups	gmane.comp.kde.kimdaba
Message-ID	<[email protected]>

Hi,

I am working on my code, again.

i have understood I should provide clean patch, each one with a
specific functionality.

Right now, my commit are quite a mess, but I think I have learn
git enough to clean that.

Would you accept my patches if i provide :

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

- A patch allowing to activate or deactivate the generation of
the files used in the gallery (the index files and the html
image files)

- A patch allowing to create a JS database when exporting an
HTML gallery

- A patch with my gallery using the JS database ?

i will also correct the points Johannes gives me to correct as
much as I can.

Cheers,

Samuel Kay

PS : Live demo is here (not the very last version) : http://poivron-robotique.fr/Demo_KPA_export/

Samuel Kay wrote:

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

[email protected]

https://mail.kdab.com/mailman/listinfo/kphotoalbum

_______________________________________________

KPhotoAlbum mailing list

[email protected]

https://mail.kdab.com/mailman/listinfo/kphotoalbum

_______________________________________________
KPhotoAlbum mailing list
[email protected]
https://mail.kdab.com/mailman/listinfo/kphotoalbum