[Nepomuk] Re: [kde-runtime/nepomuk/writeback] nepomuk/Writeback: This is my first commit, i want to verify if the approach is correct , i will correct the mistakes after verification.
Sebastian Trüg
trueg at kde.org
Mon May 9 21:03:12 CEST 2011
Hi Smit,
some comments in the order I encounter the issues:
- files and folders in kde should always be all lower-case
- there is no need for the find_package stuff in your CMakeLists.txt.
Instead add "add_subdirectory(writeback)" to the services subdir and
move your folder there.
- the taglib includes are wrong. Instead you should use a cmake macro
that finds the taglib globally. I am sure Amarok has one. We can also
look together.
- For later: the plugin should be build into its own plugin via
kde4_add_plugin.
- seems like the writeplugin.cpp is not used.
- WritebackPlugin should not be derived from Nepomuk::Service. It
represents the base class for your own plugin system. Instead you should
have one class named something like WritebackService and that is derived
from Nepomuk::Service. This class then acts as the main class for your
service/application.
- General remark: while C++ supports multi-inheritance one class can
never be derived from two classes that share one super-class (in this
case QObject and Service. The latter itself is derived from QObject)
- Documentation: please use doxygen-style comments instead of putting
them behind the methods. Use any of the public Nepomuk classes in
kdelibs/nepomuk/query as template.
- method names never start with a capital letter. They always use camel
case. (This is KDE policy)
- File is not a class. Typically you would use KUrl to reference a local
file in KDE (Example: "void doWriteback(const KUrl& url)".
- There is no need to add the QVariantList parameter to the
WritebackPlugin::Private class. You do not use it anyway. Also the
Private class is not derived from Service.
- The NEPOMUK_EXPORT_SERVICE is correct but should go into the cpp file
of your WritebackService class.
- the example plugin is a very good start. It misses the plugin export
macro call though. And as mentioned above it should be compiled into its
own plugin. Examples can be found in nepomukannotation/plugins.
That is all for now. :)
A very good start. It looks to me as if you will make good progress. Do
not let the number of my comments give another impression.
Cheers,
Sebastian
On 05/09/2011 07:32 PM, Smit Shah wrote:
> Git commit b34ca51fe02843156432572a0de00e41c5c035fb by Smit Shah.
> Committed on 09/05/2011 at 19:31.
> Pushed by smitshah into branch 'nepomuk/writeback'.
>
> This is my first commit, i want to verify if the approach is correct , i will correct the mistakes after verification.
>
> A +47 -0 nepomuk/Writeback/CMakeLists.txt [License: UNKNOWN] *
> A +51 -0 nepomuk/Writeback/mywritebackplugin.cpp [License: GPL]
> A +37 -0 nepomuk/Writeback/mywritebackplugin.h [License: GPL]
> A +64 -0 nepomuk/Writeback/writebackplugin.cpp [License: GPL]
> A +11 -0 nepomuk/Writeback/writebackplugin.desktop [License: UNKNOWN] *
> A +80 -0 nepomuk/Writeback/writebackplugin.h [License: GPL]
> A +40 -0 nepomuk/Writeback/writebackplugin_export.h [License: GPL]
> A +59 -0 nepomuk/Writeback/writeplugin.cpp [License: GPL]
>
> The files marked with a * at the end have a non valid license. Please read: http://techbase.kde.org/Policies/Licensing_Policy and use the headers which are listed at that page.
>
>
> http://commits.kde.org/kde-runtime/b34ca51fe02843156432572a0de00e41c5c035fb
>
> diff --git a/nepomuk/Writeback/CMakeLists.txt b/nepomuk/Writeback/CMakeLists.txt
> new file mode 100644
> index 0000000..4c0d20a
> --- /dev/null
> +++ b/nepomuk/Writeback/CMakeLists.txt
> @@ -0,0 +1,47 @@
> +project(nepomukwritebackservice)
> +
> +find_package(KDE4 REQUIRED)
> +find_package(Nepomuk REQUIRED)
> +
> +add_definitions(-DDISABLE_NEPOMUK_LEGACY=1)
> +
> +include (KDE4Defaults)
> +
> +include_directories(
> + ${QT_INCLUDES}
> + ${KDE4_INCLUDES}
> + ${SOPRANO_INCLUDE_DIR}
> + ${CMAKE_SOURCE_DIR}
> + ${NEPOMUK_INCLUDE_DIR}
> + ${CMAKE_CURRENT_SOURCE_DIR}/../taglib
> + ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/toolkit
> + ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/ape
> + ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/mpeg
> + ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/mpeg/id3v1
> + ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/mpeg/id3v2
> + ${CMAKE_CURRENT_SOURCE_DIR}/../bindings/c/
> + )
> +
> +set( SRCS
> +mywritebackplugin.cpp
> +writebackplugin.cpp
> +writeplugin.cpp)
> +
> +kde4_add_plugin(nepomukwritebackservice ${SRCS})
> +
> +target_link_libraries(nepomukwritebackservice
> + ${KDE4_KDEUI_LIBS}
> + ${KDE4_KIO_LIBS}
> + ${NEPOMUK_LIBRARIES}
> + ${NEPOMUK_QUERY_LIBRARIES}
> + ${SOPRANO_LIBRARIES}
> +tag)
> +
> +install(
> + FILES nepomukwritebackservice.desktop
> + DESTINATION ${SERVICES_INSTALL_DIR})
> +
> +install(
> + TARGETS nepomukwritebackservice
> + DESTINATION ${PLUGIN_INSTALL_DIR})
> +# -----------------------------
> diff --git a/nepomuk/Writeback/mywritebackplugin.cpp b/nepomuk/Writeback/mywritebackplugin.cpp
> new file mode 100644
> index 0000000..b10b053
> --- /dev/null
> +++ b/nepomuk/Writeback/mywritebackplugin.cpp
> @@ -0,0 +1,51 @@
> +/*
> +Copyright (C) 2011 Smit Shah <Who828 at gmail.com>
> +
> +This program is free software; you can redistribute it and/or
> +modify it under the terms of the GNU General Public License as
> +published by the Free Software Foundation; either version 2 of
> +the License or (at your option) version 3 or any later version
> +accepted by the membership of KDE e.V. (or its successor approved
> +by the membership of KDE e.V.), which shall act as a proxy
> +defined in Section 14 of version 3 of the license.
> +
> +This program is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +GNU General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +#include<taglib/fileref.h>
> +#include<taglib/tag.h>
> +
> +#include<mywritebackplugin.h>
> +
> +Nepomuk::MyWritebackPlugin::MyWritebackPlugin(QObject* parent, const QVariantList&)
> +{
> +
> +
> +}
> +
> +Nepomuk::MyWritebackPlugin::~MyWritebackPlugin()
> +{
> +
> +}
> +
> +
> +void Nepomuk::MyWritebackPlugin::doWriteback(const File* file)
> +{
> + TagLib::FileRef f(file); // creating a reference of the file
> +
> + f.tag()->setAlbum("XYZ"); // just an example
> + f.tag()->setTitle("abc");
> + f.tag()->setArtist("Who");
> + f.tag()->setComment("this is the best song,ever !");
> +
> + emitFinsihed();
> +}
> +
> +
> +
> diff --git a/nepomuk/Writeback/mywritebackplugin.h b/nepomuk/Writeback/mywritebackplugin.h
> new file mode 100644
> index 0000000..63acdac
> --- /dev/null
> +++ b/nepomuk/Writeback/mywritebackplugin.h
> @@ -0,0 +1,37 @@
> +/*
> +Copyright (C) 2011 Smit Shah <Who828 at gmail.com>
> +
> +This program is free software; you can redistribute it and/or
> +modify it under the terms of the GNU General Public License as
> +published by the Free Software Foundation; either version 2 of
> +the License or (at your option) version 3 or any later version
> +accepted by the membership of KDE e.V. (or its successor approved
> +by the membership of KDE e.V.), which shall act as a proxy
> +defined in Section 14 of version 3 of the license.
> +
> +This program is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +GNU General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +#ifndef NEPOMUKWRITEBACKPLUGIN_H
> +#define NEPOMUKWRITEBACKPLUGIN_H
> +
> +#include<Nepomuk/Services>
> +namespace Nepomuk
> +{
> + class MyWritebackPlugin : pubic Nepomuk::WritebackPlugin
> + {
> + public:
> + MyWritebackPlugin(QObject* parent, const QVariantList&);
> + ~MyWritebackPlugin();
> + protected:
> + void doWriteback(const Nepomuk::File*);
> + };
> +
> +}
> +
> +#endif
> \ No newline at end of file
> diff --git a/nepomuk/Writeback/writebackplugin.cpp b/nepomuk/Writeback/writebackplugin.cpp
> new file mode 100644
> index 0000000..0c81088
> --- /dev/null
> +++ b/nepomuk/Writeback/writebackplugin.cpp
> @@ -0,0 +1,64 @@
> +/*
> +Copyright (C) 2011 Smit Shah <Who828 at gmail.com>
> +
> +This program is free software; you can redistribute it and/or
> +modify it under the terms of the GNU General Public License as
> +published by the Free Software Foundation; either version 2 of
> +the License or (at your option) version 3 or any later version
> +accepted by the membership of KDE e.V. (or its successor approved
> +by the membership of KDE e.V.), which shall act as a proxy
> +defined in Section 14 of version 3 of the license.
> +
> +This program is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +GNU General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +#include "writebackplugin.h"
> +
> +class Nepomuk::WritebackPlugin::Private
> +{
> +public:
> + Private( WritebackPlugin* parent,const QList< QVariant >& )
> + : Service(parent)
> + {
> + kDebug();
> + }
> +
> +private:
> + WritebackPlugin* q;
> +};
> +
> +
> +
> +Nepomuk::WritebackPlugin::WritebackPlugin(QObject* parent)
> + : QObject(parent),
> + d( new Private(this) )
> +{
> +}
> +
> +Nepomuk::WritebackPlugin::~WritebackPlugin()
> +{
> + delete d;
> +}
> +
> +
> +void Nepomuk::WritebackPlugin::Writeback( const File* file )
> +{
> + doWriteback( file );
> +}
> +
> +
> +void Nepomuk::WritebackPlugin::emitFinished()
> +{
> + emit finished();
> + emit finished( this );
> +}
> +
> +
> +NEPOMUK_EXPORT_SERVICE( Nepomuk::WriteBackService, "nepomukwritebackservice" )
> +
> +#include "writebackplugin.moc"
> diff --git a/nepomuk/Writeback/writebackplugin.desktop b/nepomuk/Writeback/writebackplugin.desktop
> new file mode 100644
> index 0000000..8a56d0f
> --- /dev/null
> +++ b/nepomuk/Writeback/writebackplugin.desktop
> @@ -0,0 +1,11 @@
> +[Desktop Entry]
> +Type=ServiceType
> +X-KDE-ServiceType=Nepomuk/WritebackPlugin
> +
> +Comment=Nepomuk writeback plugin
> +Comment[x-test]=xxNepomuk writeback pluginxx
> +
> +# The types of resources supported by the plugin, i.e. it can create annotations
> +# for this kind of resource. Wildcard * is allowed to indicate all resource types
> +[PropertyDef::X-Nepomuk-ResourceTypes]
> +Type=QStringList
> diff --git a/nepomuk/Writeback/writebackplugin.h b/nepomuk/Writeback/writebackplugin.h
> new file mode 100644
> index 0000000..5fe2e56
> --- /dev/null
> +++ b/nepomuk/Writeback/writebackplugin.h
> @@ -0,0 +1,80 @@
> +/*
> +Copyright (C) 2011 Smit Shah <Who828 at gmail.com>
> +
> +This program is free software; you can redistribute it and/or
> +modify it under the terms of the GNU General Public License as
> +published by the Free Software Foundation; either version 2 of
> +the License or (at your option) version 3 or any later version
> +accepted by the membership of KDE e.V. (or its successor approved
> +by the membership of KDE e.V.), which shall act as a proxy
> +defined in Section 14 of version 3 of the license.
> +
> +This program is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +GNU General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +#ifndef WRITEBACKPLUGIN_H
> +#define WRITEBACKPLUGIN_H
> +
> +
> +#include <QtCore/QObject>
> +#include<QFile>
> +
> +#include <Nepomuk/Service>
> +
> +
> +#include <kpluginfactory.h>
> +#include <kpluginloader.h>
> +
> +namespace Soprano {
> +clas Node;
> +}
> +
> +namespace Nepomuk {
> +
> +class WritebackPlugin : public QObject, Service
> +{
> + Q_OBJECT
> +
> +public:
> + WritebackPlugin( QObject* parent, const QList<QVariant>& args = QList<QVariant>());
> + virtual ~WritebackPlugin();
> +
> +Q_SIGNALS:
> +
> + void finished(); // to signal that writeback is finished
> +
> +private:
> +
> + void finished( Nepomuk::WritebackPlugin* plugin ); // emited by plugin once plugin is done, can't be called directly must use emitfinished in public.
> +
> +public Q_SLOTS:
> +
> + void Writeback( const File* file );
> +
> + protected:
> +
> + virtual void doWriteback( const File* file ); // its declared as virtual as subclasses will reimplement it based on what type of file it is.
> +
> +protected Q_SLOTS:
> +
> + void emitFinished(); // emit finish signal by the subclass to clarify that writeback is over.
> +
> +private:
> + class Private;
> + Private* const d;
> +
> + };
> +
> +} // End namespace Nepomuk
> +
> +
> +#define NEPOMUK_EXPORT_ANNOTATION_PLUGIN( classname, libname ) \
> +K_PLUGIN_FACTORY(factory, registerPlugin<classname>();) \
> +K_EXPORT_PLUGIN(factory(#libname))
> +
> +#endif
> \ No newline at end of file
> diff --git a/nepomuk/Writeback/writebackplugin_export.h b/nepomuk/Writeback/writebackplugin_export.h
> new file mode 100644
> index 0000000..0d63553
> --- /dev/null
> +++ b/nepomuk/Writeback/writebackplugin_export.h
> @@ -0,0 +1,40 @@
> +/*
> +Copyright (C) 2011 Smit Shah <Who828 at gmail.com>
> +
> +This program is free software; you can redistribute it and/or
> +modify it under the terms of the GNU General Public License as
> +published by the Free Software Foundation; either version 2 of
> +the License or (at your option) version 3 or any later version
> +accepted by the membership of KDE e.V. (or its successor approved
> +by the membership of KDE e.V.), which shall act as a proxy
> +defined in Section 14 of version 3 of the license.
> +
> +This program is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +GNU General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +#ifndef NEPOMUKWRITEBACK_EXPORT_H
> +#define NEPOMUKWRITEBACK_EXPORT_H
> +
> +/* needed for KDE_EXPORT and KDE_IMPORT macros */
> +#include <kdemacros.h>
> +
> +#ifndef NEPOMUKWRITEBACK_EXPORT
> +# if defined(MAKE_LIBNEPOMUKWRITEBACK_LIB)
> + /* We are building this library */
> +# define NEPOMUKWRITEBACK_EXPORT KDE_EXPORT
> +# else
> + /* We are using this library */
> +# define NEPOMUKWRITEBACK_EXPORT KDE_IMPORT
> +# endif
> +#endif
> +
> +# ifndef NEPOMUKWRITEBACK_EXPORT_DEPRECATED
> +# define NEPOMUKWRITEBACK_EXPORT_DEPRECATED KDE_DEPRECATED NEPOMUKWRITEBACK_EXPORT
> +# endif
> +
> +#endif
> \ No newline at end of file
> diff --git a/nepomuk/Writeback/writeplugin.cpp b/nepomuk/Writeback/writeplugin.cpp
> new file mode 100644
> index 0000000..6ef07bc
> --- /dev/null
> +++ b/nepomuk/Writeback/writeplugin.cpp
> @@ -0,0 +1,59 @@
> +/*
> +Copyright (C) 2011 Smit Shah <Who828 at gmail.com>
> +
> +This program is free software; you can redistribute it and/or
> +modify it under the terms of the GNU General Public License as
> +published by the Free Software Foundation; either version 2 of
> +the License or (at your option) version 3 or any later version
> +accepted by the membership of KDE e.V. (or its successor approved
> +by the membership of KDE e.V.), which shall act as a proxy
> +defined in Section 14 of version 3 of the license.
> +
> +This program is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +GNU General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +#include "annotationplugin.h"
> +
> +class Nepomuk::WritebackPlugin::Private
> +{
> +public:
> + Private( WritebackPlugin* parent )
> + : q( parent ) {
> + }
> +
> +private:
> + WritebackPlugin* q;
> +};
> +
> +
> +
> +Nepomuk::WritebackPlugin::WritebackPlugin(QObject* parent)
> + : QObject(parent),
> + d( new Private(this) )
> +{
> +}
> +
> +Nepomuk::WritebackPlugin::~WritebackPlugin()
> +{
> + delete d;
> +}
> +
> +
> +void Nepomuk::WritebackPlugin::Writeback( const File* file )
> +{
> + doWriteback( file );
> +}
> +
> +
> +void Nepomuk::WritebackPlugin::emitFinished()
> +{
> + emit finished();
> + emit finished( this );
> +}
> +
> +#include "writebackplugin.moc"
>
More information about the Nepomuk
mailing list