Re: Upload-Plugin

Hi,

 first of all: wow. :) I'm impressed and very happy.

On Monday 21 January 2008, Niko Sams wrote:
> I did some work on an upload plugin for Quanta4. And I'm asking you
> now to review my code - this is the first time I'm working with
> KConfig, KIO, KDevelop... 

Sincerely, I'm also behind what happened in KDevelop in the past month), so we are somewhat equal in that regard. I'm learning some stuff now as I review it.

> I tested it with quanta - but it should
> work also with kdevelop - as it has no dependencies on quanta.

That is the idea behind (some of) the plugins. :)

> Installation:
> Apply the attached upload.diff to quanta and extract upload.tar.gz.
> Additionally a small fix for quanta projectfilemanager.diff I made -
> it adds folders as folders and not files to the project.

Great, thanks for it!

> Usage:
> - open a project
> - Settings -> Configure Project
> - Upload, edit profiles there
> - rightclick a project-item, select upload

Works as advertised (but see comments at the end).

> Uploadprofiles and times of uploads will be saved in the
> developer-project-config.

I think that's ok.

> And now my questions:
> - where could a upload-action be placed? (Context-Menu for the
> project-items is not very intuitive - it should be in some menu)

I think it should be in the context menu and in the Project menu, just like in Quanta3.  If you look there, you will see the following actions:
Project menu->Upload project
Context menu when clicking on the project title->Upload Project
Context menu on a project folder->Upload Folder and Quick Upload Folder
Context menu on a project file->Upload File and Quick Upload File
Context menu on a document tab->Upload File 

The Quick Upload means it uploads to the default profile without further questions.

As you see, the menu text depends on where you request the context menu, so this is something that needs to be implemented in the plugin as well, where you have only Upload File. Maybe you'd even have to create more QActions in the requestContextMenuActions method.

>    But how does such a action know what project is active?

Well, that's a tricky question and something that will be tricky for many things, since KDevelop support now more than one open projects, as much as I was against it. :) I think it should be done as for the project config (see Settings->Configure Project). So it would be a Project->Upload project and a list for loaded projects. AFAIK there is no such notion in KDevelop platform as "active project", so if we have a default shortcut for upload, that should as which one to upload. To make it easier to the user, such questions should appear only when more than one project is loaded.
 So you'd probably need a KActionMenu based action which is plugged in the Project menu from an .rc file, something like:
<!DOCTYPE kpartgui>
<kpartplugin name=upload library="libkdevupload" version="1">
<MenuBar>
 <Menu name="project">
  <Action name="project_upload" />
 </Menu>
</MenuBar>
</kpartplugin>

This should be merged into the main Project Menu (but of course it is untested).


> - Is the model-proxy approach I made with UploadProjectModel a good
> idea? 

Yes, I think it is a good idea, this is how these kind of things should be done in Qt4.

> - three files are linked into the plugin and the pref-plugin;
> can i avoid this? 

You mean the same file is compiled into different targets? With KDE3's automake based build system I'd say this can be avoid (or better, should be avoided) by creating a static "convenience" library, which holds the common files and you link those targets against this library. AFAIK, this is not supported with cmake.  Yes, unfortunately this double inclusion cannot be avoided. See:
http://www.cmake.org/Wiki/CMake_FAQ#Does_CMake_support_.22convenience.22_libraries.3F

> - uploaddialog.cpp:176 is that working?

This one?
  connect(this, SIGNAL(finished(int)), job, SLOT(kill()));

As I understand you want to kill the job when the dialog is closed. This should work, but probably a nicer solution is to reimplement QDialog::done() and there stop/kill the job and call the parent's done slot. Especially if you need to do some more cleanup there/


> Please tell me whats good/bad about the plugin - and for what parts
> there would be a easier/better solultion...

Of course I couldn't read it line by line, and I don't want to do. For the code, I have the following (small) comments:
- please add doxygen documentation for the headers. I know it is not the case for all the other plugins, but i'd like to set this as a requirement for new code that is added.
- for kDebug() Quanta has the "24000" debug area allocated (kDebug(24000)). But as now KDE supports per-project debug area setting in the CMakeFile.txt, this is not a big issue. I'll add that one, so you can keep the kDebug() lines as they are. :)

Regarding the functionality, I noticed so far the following issues (I realize that they would be too much to ask from a first implementation, but these are what came to my mind, in random order):
- it is missing some functionality from the old dialog. I'm not sure if we really need all the button from the right (All, Modified, Invert, etc.), but I can imagine some of them made sense. They might be moved to the context menu of the treeview though.
- I find it odd that the progress bar appears only when you click Upload
- the cancel button should be below the progressbar, maybe the Upload as well, like in the old dialog
- when you create a new profile and that is the only one, the "Use as default profile "  should be ticked
- mark as modified instead of upload is needed. That's the truth, sometimes you need to do this. 
- use KWallet for storing the password
- uploading the whole project or a folder seems to not work, it gives an error like: 
Could not write to file /data/development/quanta/upload/bin/akte.part

(bin/akte is what it is trying to upload). Probably does not create the "bin" folder on the upload server.

- would be nice if the plugin would create a treeview side-widget (like the Project tree) for each upload profile and would list the content of the servers there. This was there in Quanta3.

Ok, so after these long mail the questions:
- do you have an svn account? If so, feel free to commit it (after adding the doxygen docs). If not, request one from [email protected], with my recommendation.
- do you plan to work on this (or other parts of Quanta) in the future? I hope yes. :)

Thanks for your work,
Andras


-- 
Quanta Plus developer - http://quanta.kdewebdev.org K Desktop Environment - http://www.kde.org

_______________________________________________
quanta-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/quanta-devel

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.4-svn0 (GNU/Linux)

iD8DBQBHo0NdTQdfac6L/08RAuI4AKC882OZUluLmtlkpzpjVNcNsS7t3QCeKq7I
I2JKEhNzFyMypnXunQFaUJ0=
=hXj0
-----END PGP SIGNATURE-----