Bug 2020883 - Review Request: kalendar - A calendar application using Akonadi to sync with external services
Summary: Review Request: kalendar - A calendar application using Akonadi to sync with ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Maxwell G
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-11-06 19:25 UTC by Onuralp SEZER
Modified: 2021-12-11 01:32 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-11-30 20:12:21 UTC
Type: ---
gotmax: fedora-review+


Attachments (Terms of Use)
Adds %uuid macro (1.22 KB, patch)
2021-11-24 03:17 UTC, Maxwell G
no flags Details | Diff

Description Onuralp SEZER 2021-11-06 19:25:30 UTC
Spec URL: https://pagure.io/kalendar/raw/main/f/kalendar.spec
SRPM URL: https://pagure.io/kalendar/raw/main/f/kalendar-0.1.0-1.fc35.src.rpm

Description:Kalendar is a Kirigami-based calendar application that uses Akonadi. It lets you add, edit and delete events from local and remote accounts of your choice, while keeping changes syncronised across your Plasma desktop or phone.

Fedora Account System Username: thunderbirdtr


Pagure URL : https://pagure.io/kalendar

Side note : I already know "appstream-util" disabled because of "unknown tag url" and that's looks like a upstream issue or I may be missed a part

Comment 1 Maxwell G 2021-11-12 00:03:43 UTC
Hi thunderbirdtr,

> License:        LGPLv2+

I don't think this license is correct.

> ## License
> 
> This project is licensed under GPL-3.0-or-later. Some files are licensed under
> more permissive licenses. New contributions are expected to be under the
> LGPL-2.1-or-later license.

-- https://invent.kde.org/pim/kalendar/-/blob/master/README.md

It should be changed to `GPLv3+`. This is the effective license [1]. The README states that explicitly.

Thanks,
Maxwell

[1]: https://fedoraproject.org/wiki/Licensing:FAQ#What_is_.22effective_license.22_and_do_I_need_to_know_that_for_the_License:_tag.3F

Comment 2 Maxwell G 2021-11-12 00:13:13 UTC
It also probably makes sense to add `Recommends: kdepim-runtime`, as the README recommends.

> **We also strongly recommend you install the `kdepim-runtime` package before starting Kalendar** -- this will provide you with the ability to add calendars from online resources. Having this package will also let Kalendar's backend automatically create a default local calendar.

Comment 3 Maxwell G 2021-11-12 00:33:45 UTC
Please note that I am not a sponsor/reviewer, so my comments are informal.

Please see the following rpmlint errors:

```
kalendar.x86_64: W: spelling-error %description -l en_US syncronised -> synchronized
kalendar.x86_64: W: incoherent-version-in-changelog 0.1-1 ['0.1.0-1.fc36', '0.1.0-1']
kalendar.x86_64: W: no-manual-page-for-binary kalendar
kalendar.src: W: spelling-error %description -l en_US syncronised -> synchronized
kalendar.src:66: W: macro-in-comment %{buildroot}
kalendar.src:66: W: macro-in-comment %{_metainfodir}
kalendar.src:66: W: macro-in-comment %{name}
```

The `incoherent-version-in-changelog` error is because you have a typo in your `%changelog`.

`fedora-review` also found the following two issues. See here[1] for the full output.

```
- If your application is a C or C++ application you must list a
  BuildRequires against gcc, gcc-c++ or clang.
  Note: No gcc, gcc-c++ or clang found in BuildRequires
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/
- Dist tag is present.
```

[1]: https://download.copr.fedorainfracloud.org/results/gotmax23/kalendar/fedora-rawhide-x86_64/02950820-kalendar/fedora-review/review.txt

Comment 4 marcdeop 2021-11-16 21:31:37 UTC
You can have a look at my spec and src.rpm files:

https://marcdeop.fedorapeople.org/kalendar-0.2.1-1.fc34.src.rpm
https://marcdeop.fedorapeople.org/kalendar.spec

For some reason I cannot get rid of `W: spelling-error %description -l en_US kde -> ode, deck` (no matter what filter I use, maybe a bug on rpmlint?)

Comment 5 marcdeop 2021-11-16 21:40:48 UTC
The kalendar developer reminded me that it's important we have:

Requires:       kdepim-runtime


(see my spec file as an example)

Comment 6 Maxwell G 2021-11-23 02:42:07 UTC
@thunderbirdtr

Comment 7 Maxwell G 2021-11-23 03:04:08 UTC
@thunderbirdtr@fedoraproject.org, are you interested in continuing with the review, i.e. fixing the issues and updating to the latest version? If not, Marc should create a new ticket. I am happy to review either way, but right now, I am unsure whose package to move forward with!

Thanks,
Maxwell

Comment 8 Onuralp Sezer 2021-11-23 05:38:06 UTC
I updated the spec file accordingly and add "rpmlintrc" as well.

https://pagure.io/kalendar/raw/main/f/kalendar.rpmlintrc

Comment 9 Maxwell G 2021-11-24 03:16:23 UTC
> # appstream-util validate-relax --nonet %{buildroot}%{_metainfodir}/org.kde.%{name}.appdata.xml

You should re-enable this. It looks like they fixed it[2].

You should also add `BuildRequires: cmake(KF5DBusAddons)`[3] to be explicit.

I attached a diff that you can consider adopting; it adds a %uuid variable. It's a matter of personal preference how much you use macros.

[2]: https://invent.kde.org/pim/kalendar/-/commit/1ec5c7ff82ad24ca70c7f7a55993daa00663dad6
[3]: https://invent.kde.org/pim/kalendar/-/compare/v0.1.0...v0.2.1?from_project_id=7106#9a2aa4db38d3115ed60da621e012c0efc0172aae

Comment 10 Maxwell G 2021-11-24 03:17:41 UTC
Created attachment 1843288 [details]
Adds %uuid macro

Comment 11 Maxwell G 2021-11-24 03:20:10 UTC
I approved your package, with the condition that you fix those two issues. You can consider adopting my patch.

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
See above

===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
     Note: Using prebuilt packages
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "*No copyright* Creative Commons CC0 1.0", "BSD 2-Clause
     License", "Unknown or generated", "*No copyright* BSD 3-Clause
     License", "*No copyright* GNU General Public License, Version 3
     Creative Commons CC0 1.0", "Creative Commons CC0 1.0", "*No copyright*
     GNU Library General Public License, Version 2.0", "BSD 3-Clause
     License", "GNU General Public License, Version 2", "GNU General Public
     License, Version 3", "GNU Library General Public License, Version
     2.0", "GNU Lesser General Public License, Version 2.1", "GNU Lesser
     General Public License, Version 3", "*No copyright* GNU General Public
     License, Version 3", "*No copyright* GNU Lesser General Public
     License, Version 2.1", "*No copyright* GNU Lesser General Public
     License, Version 3", "*No copyright* GNU General Public License GNU
     Lesser General Public License, Version 2.1", "*No copyright* GNU
     General Public License, Version 2". 25 files have unknown license.
     Detailed output of licensecheck in /home/gotmax/Sync/git-
     repos/packaging/fedora_rpms/review.repos/kalendar/thunderbirdtr/kalendar/results_kalendar/0.2.1/1.fc35/kalendar/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[!]: Package must own all directories that it creates.
     Note: Directories without known owners:
     /usr/share/icons/hicolor/scalable,
     /usr/share/icons/hicolor/scalable/apps, /usr/share/icons/hicolor

     I think you should add 'Requires: hicolor-icon-theme`. Admittedly, I'm not super sure about this issue.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: The spec file handles locales properly.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 1 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install or
     desktop-file-validate if there is such a file.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: Reviewer should test that the package builds in mock.
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[?]: Package should compile and build into binary rpms on all supported
     architectures.
[!]: %check is present and all tests pass.
    Fix appstream-util line
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: There are rpmlint messages (see attachment).
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.


Rpmlint
-------
Cannot parse rpmlint output:


Rpmlint (debuginfo)
-------------------
Cannot parse rpmlint output:



Rpmlint (installed packages)
----------------------------
Cannot parse rpmlint output:


Source checksums
----------------
https://download.kde.org/stable/kalendar/kalendar-0.2.1.tar.xz :
  CHECKSUM(SHA256) this package     : d432460bb456cb3624b8ca1becbb6417ebe952b34648975c04b16aa9ce203398
  CHECKSUM(SHA256) upstream package : d432460bb456cb3624b8ca1becbb6417ebe952b34648975c04b16aa9ce203398


Requires
--------
kalendar (rpmlib, GLIBC filtered):
    akonadi-calendar-tools
    kdepim-addons
    kdepim-runtime
    kf5-kirigami2
    kf5-kirigami2-addons
    kf5-kirigami2-addons-treeview
    libKF5AkonadiCalendar.so.5()(64bit)
    libKF5AkonadiContact.so.5()(64bit)
    libKF5AkonadiCore.so.5()(64bit)
    libKF5AkonadiWidgets.so.5()(64bit)
    libKF5CalendarCore.so.5()(64bit)
    libKF5CalendarSupport.so.5()(64bit)
    libKF5CalendarUtils.so.5()(64bit)
    libKF5Codecs.so.5()(64bit)
    libKF5ConfigCore.so.5()(64bit)
    libKF5ConfigGui.so.5()(64bit)
    libKF5ConfigWidgets.so.5()(64bit)
    libKF5Contacts.so.5()(64bit)
    libKF5CoreAddons.so.5()(64bit)
    libKF5DBusAddons.so.5()(64bit)
    libKF5EventViews.so.5()(64bit)
    libKF5I18n.so.5()(64bit)
    libKF5ItemModels.so.5()(64bit)
    libKF5WidgetsAddons.so.5()(64bit)
    libKF5WindowSystem.so.5()(64bit)
    libKF5XmlGui.so.5()(64bit)
    libQt5Core.so.5()(64bit)
    libQt5Core.so.5(Qt_5)(64bit)
    libQt5Core.so.5(Qt_5.15)(64bit)
    libQt5Gui.so.5()(64bit)
    libQt5Gui.so.5(Qt_5)(64bit)
    libQt5Qml.so.5()(64bit)
    libQt5Qml.so.5(Qt_5)(64bit)
    libQt5Quick.so.5()(64bit)
    libQt5Quick.so.5(Qt_5)(64bit)
    libQt5Widgets.so.5()(64bit)
    libQt5Widgets.so.5(Qt_5)(64bit)
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgcc_s.so.1(GCC_3.3.1)(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    rtld(GNU_HASH)

kalendar-debuginfo (rpmlib, GLIBC filtered):

kalendar-debugsource (rpmlib, GLIBC filtered):



Provides
--------
kalendar:
    application()
    application(org.kde.kalendar.desktop)
    kalendar
    kalendar(x86-64)
    metainfo()
    metainfo(org.kde.kalendar.appdata.xml)
    mimehandler(text/calendar)

kalendar-debuginfo:
    debuginfo(build-id)
    kalendar-debuginfo
    kalendar-debuginfo(x86-64)

kalendar-debugsource:
    kalendar-debugsource
    kalendar-debugsource(x86-64)



Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10
Command line :/usr/bin/fedora-review -p --name kalendar --rpm-spec --no-build
Buildroot used: fedora-rawhide-x86_64
Active plugins: Shell-api, C/C++, Generic
Disabled plugins: Python, Ocaml, R, Perl, Haskell, SugarActivity, Java, fonts, PHP
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 12 Maxwell G 2021-11-24 14:31:31 UTC
Hi Onuralp,

I have a couple more pointers. I will approve your package once everything is fixed. In retrospect, I should have done that in the first place.

(In reply to Maxwell G from comment #11)
> [!]: Package must own all directories that it creates.
>     Note: Directories without known owners:
>     /usr/share/icons/hicolor/scalable,
>     /usr/share/icons/hicolor/scalable/apps, /usr/share/icons/hicolor
>
>     I think you should add 'Requires: hicolor-icon-theme`. Admittedly, I'm not super sure about this issue.

I confirmed that you need to own those icon directories. One option is to add `Requires: hicolor-icon-theme`. You can alternatively put `%dir $unowned_directory` in the `%files` section for each of the unowned directories. I prefer the former, but both solutions are valid.

> appstream-util validate-relax --nonet %{buildroot}%{_metainfodir}/org.kde.%{name}.appdata.xml

> %files -f %{name}.lang
> [...]
> %{_kf5_datadir}/metainfo/org.kde.kalendar.appdata.xml 
> [...]
 
You should use %{_kf5_metainfodir}[1] instead of `%{_metainfodir}` or `%{_kf5_datadir}/metainfo`.

> License:        GPLv3+

I don't think this is required, but I recommend adding a comment above the `License` field in your specfile linking to or paraphrasing my previous comment about licensing[2] to avoid any confusion down the line.

Thanks,
Maxwell

[1]: https://src.fedoraproject.org/rpms/kf5/blob/rawhide/f/macros.kf5#_9
[2]: https://bugzilla.redhat.com/show_bug.cgi?id=2020883#c1

Comment 13 Onuralp SEZER 2021-11-29 19:05:27 UTC
(In reply to Maxwell G from comment #12)
> Hi Onuralp,
> 
> I have a couple more pointers. I will approve your package once everything
> is fixed. In retrospect, I should have done that in the first place.
> 
> (In reply to Maxwell G from comment #11)
> > [!]: Package must own all directories that it creates.
> >     Note: Directories without known owners:
> >     /usr/share/icons/hicolor/scalable,
> >     /usr/share/icons/hicolor/scalable/apps, /usr/share/icons/hicolor
> >
> >     I think you should add 'Requires: hicolor-icon-theme`. Admittedly, I'm not super sure about this issue.

I added hicolor-icon-theme instead of owning (easier and cleaner)

> I confirmed that you need to own those icon directories. One option is to
> add `Requires: hicolor-icon-theme`. You can alternatively put `%dir
> $unowned_directory` in the `%files` section for each of the unowned
> directories. I prefer the former, but both solutions are valid.
> 
> > appstream-util validate-relax --nonet %{buildroot}%{_metainfodir}/org.kde.%{name}.appdata.xml
> 
> > %files -f %{name}.lang
> > [...]
> > %{_kf5_datadir}/metainfo/org.kde.kalendar.appdata.xml 
> > [...]

kf5_metainfodir done

> You should use %{_kf5_metainfodir}[1] instead of `%{_metainfodir}` or
> `%{_kf5_datadir}/metainfo`.


I'm gonna keep it 

> > License:        GPLv3+
> 
> I don't think this is required, but I recommend adding a comment above the
> `License` field in your specfile linking to or paraphrasing my previous
> comment about licensing[2] to avoid any confusion down the line.
> 
> Thanks,
> Maxwell
> 
> [1]: https://src.fedoraproject.org/rpms/kf5/blob/rawhide/f/macros.kf5#_9
> [2]: https://bugzilla.redhat.com/show_bug.cgi?id=2020883#c1

Comment 15 Onuralp SEZER 2021-11-29 19:08:35 UTC
This is also fixed as well. (It is enabled)

appstream-util validate-relax --nonet %{buildroot}%{_metainfodir}/org.kde.%{name}.appdata.xml

Comment 16 Maxwell G 2021-11-29 22:38:23 UTC
Hi Onuralp,

We are almost there; there are just a couple minor issues left.

(In reply to Maxwell G from comment #9)
> You should also add `BuildRequires: cmake(KF5DBusAddons)`[3] to be explicit.
> 
> [3]: https://invent.kde.org/pim/kalendar/-/compare/v0.1.0...v0.2.1?from_project_id=7106#9a2aa4db38d3115ed60da621e012c0efc0172aae

This still needs to be fixed.

> %check
> desktop-file-validate %{buildroot}%{_datadir}/applications/org.kde.%{name}.desktop
> appstream-util validate-relax --nonet %{buildroot}%{_metainfodir}/org.kde.%{name}.appdata.xml

According to the KDE SIG,`%{_kf5_*}` macros should be used for KDE applications[1,2]. In your case, you should use `%{_kf5_datadir}` instead of `%{_datadir}` and `%{_kf5_metainfodir}` instead of `%{_metainfodir}`. For the aforementioned macros, the kf5 variants actually have the same values as the non kf5 variants in Fedora, but the kf5 macros are still preferred. Also, you should remain consistent; don't use `%{_datadir}` in one part of the spec and `%{_kf5_datadir}` in another. Admittedly, this is an unimportant change, but I don't think it requires much effort to implement, so we should do so.

Thanks,
Maxwell

[1]: https://lists.fedoraproject.org/archives/list/kde@lists.fedoraproject.org/thread/ZPZTD4F77OICVSPIL7BL22GGQXWQM7PL/
[2]: https://lists.fedoraproject.org/archives/list/kde@lists.fedoraproject.org/thread/A4B7MUWO3QYKNA26V2FIEP73C4RT6IJU/

Comment 17 Onuralp SEZER 2021-11-30 06:02:55 UTC
(In reply to Maxwell G from comment #16)
> Hi Onuralp,
> 
> We are almost there; there are just a couple minor issues left.

cmake(KF5DBusAddons) is added, thank you very much.

> (In reply to Maxwell G from comment #9)
> > You should also add `BuildRequires: cmake(KF5DBusAddons)`[3] to be explicit.
> > 
> > [3]: https://invent.kde.org/pim/kalendar/-/compare/v0.1.0...v0.2.1?from_project_id=7106#9a2aa4db38d3115ed60da621e012c0efc0172aae
> 
> This still needs to be fixed.

kf5 macros used.
And uuid section added as well. 

> 
> > %check
> > desktop-file-validate %{buildroot}%{_datadir}/applications/org.kde.%{name}.desktop
> > appstream-util validate-relax --nonet %{buildroot}%{_metainfodir}/org.kde.%{name}.appdata.xml
> 
> According to the KDE SIG,`%{_kf5_*}` macros should be used for KDE
> applications[1,2]. In your case, you should use `%{_kf5_datadir}` instead of
> `%{_datadir}` and `%{_kf5_metainfodir}` instead of `%{_metainfodir}`. For
> the aforementioned macros, the kf5 variants actually have the same values as
> the non kf5 variants in Fedora, but the kf5 macros are still preferred.
> Also, you should remain consistent; don't use `%{_datadir}` in one part of
> the spec and `%{_kf5_datadir}` in another. Admittedly, this is an
> unimportant change, but I don't think it requires much effort to implement,
> so we should do so.
> 
> Thanks,
> Maxwell
> 
> [1]:
> https://lists.fedoraproject.org/archives/list/kde@lists.fedoraproject.org/
> thread/ZPZTD4F77OICVSPIL7BL22GGQXWQM7PL/
> [2]:
> https://lists.fedoraproject.org/archives/list/kde@lists.fedoraproject.org/
> thread/A4B7MUWO3QYKNA26V2FIEP73C4RT6IJU/

Comment 18 Onuralp SEZER 2021-11-30 06:06:36 UTC
(In reply to Maxwell G from comment #2)
> It also probably makes sense to add `Recommends: kdepim-runtime`, as the
> README recommends.
> 
> > **We also strongly recommend you install the `kdepim-runtime` package before starting Kalendar** -- this will provide you with the ability to add calendars from online resources. Having this package will also let Kalendar's backend automatically create a default local calendar.

https://pagure.io/kalendar/c/f660fa8cb14b15be448ee9021468bd3a3647e2f1?branch=main 

This is added as well.

Comment 19 Onuralp SEZER 2021-11-30 06:08:24 UTC
(In reply to Onuralp SEZER from comment #18)
> (In reply to Maxwell G from comment #2)
> > It also probably makes sense to add `Recommends: kdepim-runtime`, as the
> > README recommends.
> > 
> > > **We also strongly recommend you install the `kdepim-runtime` package before starting Kalendar** -- this will provide you with the ability to add calendars from online resources. Having this package will also let Kalendar's backend automatically create a default local calendar.
> 
> https://pagure.io/kalendar/c/
> f660fa8cb14b15be448ee9021468bd3a3647e2f1?branch=main 
> 
> This is added as well.

I removed it, (sorry for duplication)

Comment 20 Maxwell G 2021-11-30 15:29:57 UTC
(In reply to Onuralp SEZER from comment #19)
> (In reply to Onuralp SEZER from comment #18)
> > (In reply to Maxwell G from comment #2)
> > > It also probably makes sense to add `Recommends: kdepim-runtime`, as the
> > > README recommends.
> > > 
> > > > **We also strongly recommend you install the `kdepim-runtime` package before starting Kalendar** -- this will provide you with the ability to add calendars from online resources. Having this package will also let Kalendar's backend automatically create a default local calendar.
> > 
> > https://pagure.io/kalendar/c/
> > f660fa8cb14b15be448ee9021468bd3a3647e2f1?branch=main 
> > 
> > This is added as well.
> 
> I removed it, (sorry for duplication)

No problem. It looks like it should be `Requires:` instead of `Recommends:` anyways. `

Comment 21 Maxwell G 2021-11-30 15:30:29 UTC
(In reply to Onuralp SEZER from comment #17)
> (In reply to Maxwell G from comment #16)
> > Hi Onuralp,
> > 
> > We are almost there; there are just a couple minor issues left.
> 
> cmake(KF5DBusAddons) is added, thank you very much.
> 
> > (In reply to Maxwell G from comment #9)
> > > You should also add `BuildRequires: cmake(KF5DBusAddons)`[3] to be explicit.
> > > 
> > > [3]: https://invent.kde.org/pim/kalendar/-/compare/v0.1.0...v0.2.1?from_project_id=7106#9a2aa4db38d3115ed60da621e012c0efc0172aae
> > 
> > This still needs to be fixed.
> 
> kf5 macros used.
> And uuid section added as well. 
> 
> > 
> > > %check
> > > desktop-file-validate %{buildroot}%{_datadir}/applications/org.kde.%{name}.desktop
> > > appstream-util validate-relax --nonet %{buildroot}%{_metainfodir}/org.kde.%{name}.appdata.xml
> > 
> > According to the KDE SIG,`%{_kf5_*}` macros should be used for KDE
> > applications[1,2]. In your case, you should use `%{_kf5_datadir}` instead of
> > `%{_datadir}` and `%{_kf5_metainfodir}` instead of `%{_metainfodir}`. For
> > the aforementioned macros, the kf5 variants actually have the same values as
> > the non kf5 variants in Fedora, but the kf5 macros are still preferred.
> > Also, you should remain consistent; don't use `%{_datadir}` in one part of
> > the spec and `%{_kf5_datadir}` in another. Admittedly, this is an
> > unimportant change, but I don't think it requires much effort to implement,
> > so we should do so.
> > 
> > Thanks,
> > Maxwell
> > 
> > [1]:
> > https://lists.fedoraproject.org/archives/list/kde@lists.fedoraproject.org/
> > thread/ZPZTD4F77OICVSPIL7BL22GGQXWQM7PL/
> > [2]:
> > https://lists.fedoraproject.org/archives/list/kde@lists.fedoraproject.org/
> > thread/A4B7MUWO3QYKNA26V2FIEP73C4RT6IJU/

Great! Thank you.

Comment 22 Maxwell G 2021-11-30 15:38:46 UTC
I approved your package. When you import this package, please make sure that you add `Fixes rhbz#2020883` to the rpm changelog and/or manually add this bug to the Bodhi update so it gets marked as CLOSED.

I also added `kalendar` to release-montioring.org (AKA Antiya)[1]. Antiya will create a Bugzilla bug against your package for each new upstream release so you can make sure your package is up to date.

Please let me know if you have any questions.

Thanks,
Maxwell

[1]: https://release-monitoring.org/project/241436/

Comment 23 marcdeop 2021-11-30 16:17:34 UTC
(In reply to Maxwell G from comment #22)
> I approved your package. When you import this package, please make sure that
> you add `Fixes rhbz#2020883` to the rpm changelog and/or manually add this
> bug to the Bodhi update so it gets marked as CLOSED.
> 
> I also added `kalendar` to release-montioring.org (AKA Antiya)[1]. Antiya
> will create a Bugzilla bug against your package for each new upstream
> release so you can make sure your package is up to date.
> 
> Please let me know if you have any questions.
> 
> Thanks,
> Maxwell
> 
> [1]: https://release-monitoring.org/project/241436/

Thanks Maxwell for all the hard work.

I would like to ask @Onuralp to give @KDE-Sig admin permissions on the repository so we could contribute in the future :-)

Comment 24 Maxwell G 2021-11-30 17:08:03 UTC
(In reply to marcdeop from comment #23)
> (In reply to Maxwell G from comment #22)
> > I approved your package. When you import this package, please make sure that
> > you add `Fixes rhbz#2020883` to the rpm changelog and/or manually add this
> > bug to the Bodhi update so it gets marked as CLOSED.
> > 
> > I also added `kalendar` to release-montioring.org (AKA Antiya)[1]. Antiya
> > will create a Bugzilla bug against your package for each new upstream
> > release so you can make sure your package is up to date.
> > 
> > Please let me know if you have any questions.
> > 
> > Thanks,
> > Maxwell
> > 
> > [1]: https://release-monitoring.org/project/241436/
> 
> Thanks Maxwell for all the hard work.
> 
> I would like to ask @Onuralp to give @KDE-Sig admin permissions on the
> repository so we could contribute in the future :-)

No problem, Marc. If I review any KDE packages in the future, should I also ask the reviewee to do that?

Comment 25 Gwyn Ciesla 2021-11-30 17:46:15 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/kalendar

Comment 26 Fedora Update System 2021-11-30 20:11:09 UTC
FEDORA-2021-25be101ebe has been submitted as an update to Fedora 36. https://bodhi.fedoraproject.org/updates/FEDORA-2021-25be101ebe

Comment 27 Fedora Update System 2021-11-30 20:12:21 UTC
FEDORA-2021-25be101ebe has been pushed to the Fedora 36 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 28 Fedora Update System 2021-11-30 21:51:00 UTC
FEDORA-2021-3dedb719ce has been submitted as an update to Fedora 35. https://bodhi.fedoraproject.org/updates/FEDORA-2021-3dedb719ce

Comment 29 Fedora Update System 2021-12-01 01:37:07 UTC
FEDORA-2021-3dedb719ce has been pushed to the Fedora 35 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --advisory=FEDORA-2021-3dedb719ce`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-3dedb719ce

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 30 marcdeop 2021-12-01 06:39:01 UTC
(In reply to Maxwell G from comment #24)
> No problem, Marc. If I review any KDE packages in the future, should I also
> ask the reviewee to do that?

In general, yes (although don't really know how much of a "rule" we can make of that).

There wasn't need of such a thing in the past in general because the main three contributors to kde-sig (Rex, Neal and Jan) are also proven packagers. However, there are contributers such as myself or Onuralp who are not ;-)

Comment 31 Fedora Update System 2021-12-03 01:49:51 UTC
FEDORA-2021-08b73cf003 has been pushed to the Fedora 35 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-08b73cf003 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-08b73cf003

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 32 Fedora Update System 2021-12-11 01:32:26 UTC
FEDORA-2021-08b73cf003 has been pushed to the Fedora 35 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.