https://bugzilla.redhat.com/show_bug.cgi?id=1982635
--- Comment #3 from Christopher Engelhard <ce(a)lcts.de> ---
Thanks for the review. I'm working on the test issue and will set NEEDINFO once
I have a new build including all fixes.
(In reply to Otto Urpelainen from comment #1)
2.
From fedora-review:
Note: Directories without known owners: /usr/share/php/League
This is a MUST item in the guidelines,
so it must be fixed.
I am not sure how vendor directory ownership is usually handed in php
packages.
I suppose all packages from League could share the ownership?
Then this would do it: %dir %{_datadir}/php/League
Reference:
https://docs.fedoraproject.org/en-US/packaging-guidelines/
#_file_and_directory_ownership
Yes. Other packages in the \League namespace already own /usr/share/php/League,
but since this one doesn't depend on any of them (unlike e.g. php-league-uri)
it should own it as well. Will fix.
> # Please preserve the changelog entries
I do not understand this request in combination with %autochangelog
Once the package is actually build, rpmautospec will replace the macro with the
actually (build from git) changelog. Hence, anybody messing around with a
specfile they got from a source package should still preserve that changelog if
possible.
3.
Guidelines say
"The composer.json file is not used,
and should be installed as %doc
as it provides useful information about the package and its dependencies.".
I am not certain how to interpret this statement.
Since it says should, I do not think it is a blocking issue.
However, it clearly recommends installing the file as %doc
instead of dropping it completely
like is done here.
Perhaps it could be added as %doc as suggested?
This is an oversight on my part. The composer file is useful (even if you don't
use composer) because it e.g. tells the user which namespaces the PHP classes
are in, where to find further documentation etc. I'll include it.
4.
> : No tests implemented
I'll include the tests by using a git checkout
instead of the prepare tarball,
as discussed here
https://bugzilla.redhat.com/show_bug.cgi?id=1982616
(In reply to Otto Urpelainen from comment #2)
1.
A release is packaged, but it downloaded by commit id rather than tag or
GitHub release. Nothing wrong with that, I am just curious why it is done
like that here?
Because people most often look for releases etc. via
composer/packagist.org,
and that references packages via commit id:
https://packagist.org/packages/beberlei/assert
Packaging it this way a) makes life easier because all composer packages look
very similar and b) ensures that no matter what upstream does in their git
repo, the package people get via composer is the same they get via the repos.
2.
> Autoloader: /usr/share/php/League/UriInterfaces/autoload.php
I am surprised that this information is important enough
to be added to description.
Why?
Because you need to know where the autoloader is in order to actually use this
package in a project, and its location standardized. No searchpath,
unfortunately.
Also, there would be a slight readability gain
from formatting this differently,
so it would not look so much like a specfile tag.
Good point, I hadn't noticed that.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component