I'm unimpressed by the quality of the code: https://github.com/systemd-cron/systemd-cron/blob/master/src/bin/crontab.cpp The time might be better spent checking return codes for errors, adding license information, writing a proper configure script etc.

I just maintain the fedora package, but I am not much involved into the actual source code development itself.

Some of the code is bad, and useless. But the bad code was before the generator was written, when the exact same repo had a primitive cron-weekly.{timer,service}-type which serialized cronjobs; It was a dirty hack.
It is nowhere used now, and might not even work. It is still left in the repo, but recommended against by the upstream developers.
This hack is disabled by default [--enable-runparts=no to ./configure to be explicit], although the code still exists in the repo.
The new clean systemd generator part is what I want to package.

The crontab binary is a relic from the pre-generator times in the repo, and just faced a c++ rewrite without much refactoring or bugfixing.
crontab.cpp is I presume the source for /usr/bin/crontab. Yes it isn't impressive, but it works well.
I don't think it needs to be impressive, as it is performant and does it's job well.
Please, if you have a problem with it, re-inform me back.

I would also like to let you know that I have tested the compiled package, and it works perfectly for my cronjobs and many other test cronjobs.

Could you elaborate on the return-code and licensing issues? I can try to fix them.

I don't think the configure script would be too much of an issue. It works fine, and isn't exactly sphagetti code.
It isn't used for runtime anyways. Just to enable some features and disable the historic "run-parts" hack in our case. At pre-compile-time only.

If you have doubts of it's motive when systemd.timer already exists, it is just a systemd generator using systemd.timer under the hood.

A single entry in crontab or just dropping in your script into a cron.daily directory is just easier when compared to totally 10+ lines in ${yourjob}.{timer,service} in addition to the line or script, especially when you have quite a few such jobs.
However, cronie/anacron are separate daemons, with redundant duplicated code, while systemd-cron uses clean well-maintained existing functionality integrated into systemd.
Using systemd-cron over cronie/anacron also means that since the executed line runs as a systemd service, the traditional envvar, logging, and resource-control issues prevalent with cron are also fixed.

Regards, Kindly fulfill my request...

I'm unimpressed by the quality of your review. Time may be better spent posting your analysis to the bug tracker, helping the sponsored maintainer, etc.

As for the licence (and knowing nothing about fedora packaging): https://packages.fedoraproject.org/pkgs/moreutils/moreutils/fedora-rawhide.html lists a COPYING file under /usr/share/doc/$pkg, but I don't see the LICENSE copied thusly in https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2350109-systemd-cron/fedora-rawhide-x86_64/08730247-systemd-cron/

There is a %license LICENSE in %files.

Do you mean that I need to copy it to the particular /usr/share/doc/%{name}/?
Along with README?
Kindly clarify if COPYING is the license itself or something else.

Thanks for letting me know.

I don't know if you need to copy it, but this is what the other package does so it's a solid guess. In that case COPYING is the licence text itself, like LICENSE in sd-cron.

The README and CHANGELOG next to %doc in %files are copied to the correct location.

BTW, how can I give you read access to my copr, just so that you may check it?

I have done cp LICENSE COPYING and in files next to %doc added COPYING.

Now it should be correctly placed, just like CHANGELOG and README.md.

[Haven't removed %license LICENSE]

Additionally, also disabled --enable-boot=no, as per upstream contrib/census.txt.
[This I later reversed to enable /etc/cron.boot anyway.]

Rebuilding the copr and re-installing the package on my system... will be back with whether it works or not soon.

Note: I am not yet sponsored.

I don't know if you need to copy it, but this is what the other package does so it's a solid guess.

It's not a "solid guess", because the package "moreutils" is not following the packaging guidelines. It still uses old-style %doc to include the license file and ought to be updated to use %license instead: https://src.fedoraproject.org/rpms/moreutils/blob/rawhide/f/moreutils.spec#_117

I have included the license in both.

%license LICENSE is the original license file from upstream.
cp LICENSE COPYING makes a copy of it for the moreutils-style license.
%doc has README and CHANGELOG already, and I added COPYING to it.
Finally, %doc README.md CHANGELOG COPYING.
I didn't remove %license LICENSE.

So it complies to both the old and new specification.

I will remove the COPYING once the package is reviewed and into fedora repos.

Note: Updated package has COPYING, enables /etc/cron.boot anyways despite fedora cron's defaults.

So it complies to both the old and new specification.

This is going into a wrong direction, because duplicating the license file is not needed.

Let it be duplicated for now. I will remove it soon.

I want to be a packager. To maintain the package. Like stated by fedora docs. https://docs.fedoraproject.org/en-US/package-maintainers/New_Package_Process_for_New_Contributors/

I have applied for package review over on bugzilla [Link in original isssue post, just after username].

I just want to be sponsored to be a "packager" for maintaining the systemd-cron package. [AND I plan to package even more, like the starship terminal prompt and patheon-DE packages.]

Thanks for your interest in contributing to Fedora. Would be happy to review a Pantheon desktop related package.

If you plan to work on Pantheon related things, please coordinate with me and the Pantheon SIG. There is already ongoing work wrt/ adding back Pantheon DE packages to Fedora.

Same for starship - please coordinate with the Rust SIG. The previous maintainer couldn't spend the time required to keep it up-to-date, which is why it was previously removed from Fedora again.

Yes, but 1st I want to package systemd-cron only, and get sponsored.

Then I'll package starship.

I will then start with elementaryOS. [Maybe even COSMIC, but I don't think I'll actually touch it till it's sufficiently post-alpha.]

[I want to package pantheon packages just to help in creating the sodalite successor.]

Fedora is a volunteer effort. Shouting at people doesn't make them more motivated.

Fedora is a volunteer effort. Shouting at people doesn't make them more motivated.

Sorry,

I deleted the "shouting" post.

Will patiently wait....

BTW, I am here to be a [maybe small and insignificant, but still...] part of the volunteer effort at maintaining and improving fedora.
[If you think "Your silly package doesn't do anything", it's a cron implementation, which doesn't duplicate systemd-timers, unlike cronie/anacron which re-implemented all the timer code and had to be an ever-running redundant daemon, unsuitable for being on fedora installs.]
[I don't plan for this to be in fedora default installation, but I want it to be remotely suitable.]

I didn't read the full discussion, but I want to point out two things:

1. This ticket is not the right place for doing a package review. That should be done in the package review Bugzilla ticket or in a 1on1 off-list discussion. The purpose of a ticket in this repository is for a contributor to find a sponsor after their first package review is finished
2. AFAIK the package reviewer is not supposed to judge the quality of the upstream code but only the quality of the specfile

I'm currently reviewing pramodvu1502 his package request at https://bugzilla.redhat.com/show_bug.cgi?id=2350109

I notice his impatience which is kind of understandable reading the above messages.

Besides that I also see his commitment. Please consider making him a package maintainer.

Pramodvu, please note that most of us (me included) are volunteers, be patient.

I'm currently reviewing pramodvu1502 his package request at https://bugzilla.redhat.com/show_bug.cgi?id=2350109

I notice his impatience which is kind of understandable reading the above messages.

Besides that I also see his commitment. Please consider making him a package maintainer.

Pramodvu, please note that most of us (me included) are volunteers, be patient.

I don't see that he meets the requirements to be sponsored, his package is a mess.

It doesn't use the macros and has no changelog, also I don't like his attitude.

I don't see that he meets the requirements to be sponsored, his package is a mess.

Kindly suggest improvements too... I can improve it, am improving it.

It doesn't use the macros

Like? %systemd_post doesn't seem to work (I tested it long ago; If you think that it surely does, let me know)

As for the systemctl reload-or-restart call to systemd-sysusers and systemd-tmpfiles, sorry... Suggestions needed here... whether that is required or not...

has no changelog
I am fixing it... new specfile on the way. Just a few minutes... EDIT: already done

I don't like his attitude.

Sorry.
I don't think that should affect the package though...

I noticed that as well but I'd like to give Pramodvu a chance to improve, giving vague hints here doesn't help. Please do as frostyx said and give actionable feedback in bugzilla.

Like? %systemd_post doesn't seem to work (I tested it long ago; If you think that it surely does, let me know)

That's not helpful. "Long ago" means you should/could have revisited your issue by testing whether anything has changed, because afterall those macros are documented in the packaging guidelines. Also, "doesn't work" as a description isn't helpful either.
- https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_systemd
- https://fedoraproject.org/wiki/Packaging:Systemd#Why_don't_we....

I don't like his attitude.

Sorry.
I don't think that should affect the package though...

Well, I've informed you about the license file guidelines, for example, and you've outright refused to fix the package. Typically, active and potential reviewers find that off-putting.

Well, I've informed you about the license file guidelines,

That's already fixed... %license is present in %files...

That's not helpful. "Long ago" means you should/could have revisited your issue by testing whether anything has changed, because after all those macros are documented in the packaging guidelines. Also, "doesn't work" as a description isn't helpful either.

Tested it, it does work, using it.

And I also replaced all paths in the spec-file to use macros...

As for enabling the cron.target, the reason is to closely follow behaviour of cron daemons...
AND it is written to the %{_presetdir}/50-systemd-cron.preset file.