Spec URL: https://download.copr.fedorainfracloud.org/results/gui1ty/reviews/fedora-rawhide-x86_64/05788852-python-fvs/python-fvs.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/gui1ty/reviews/fedora-rawhide-x86_64/05788852-python-fvs/python-fvs-0.3.4-1.fc39.src.rpm Description: File Versioning System with hash comparison and data storage to create unlinked states that can be deleted. Why FVS? The main reason for this project is for the purpose of personal knowledge and understanding of the versioning system. The second reason is to make a simple and easy-to-implement versioning system for Bottles. There are plenty of other versioning systems out there, but all of these provide features that I wouldn't need in my projects. The purpose of FVS is to always remain as clear and simple as possible, providing only the functionality of organizing file versions into states, ie recovery points that take advantage of deduplication to minimize space consumption Fedora Account System Username: gui1ty Copr build: https://copr.fedorainfracloud.org/coprs/gui1ty/reviews/build/5788852/ This package is required by Bottles and depends on the recently packaged python-orjson.
I'd advise using the literal values everywhere instead of `%pypi_name` and `%fed_name`. Those macros make the specfile way less readable. That's not a blocker, though. Also, there's a BuildRequires on pytest even though that's not used at all. It should be removed. --- Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated - [x] The latest version is packaged or packaging an earlier version is justified. - [x] The License tag reflects the package contents and uses the correct identifiers. - [x] The package builds successfully in mock. - [x] Package is installable (checked by fedora-review). - [!] There are no relevant rpmlint errors. python3-fvs.noarch: E: summary-too-long File Versioning System with hash comparison and data storage to create unlinked states that can be deleted This should be shortened to < 80 characters. - [-] The package runs tests in %check. Upstream does not provide unit tests. I'd link to https://github.com/mirkobrombin/FVS/issues/2 in your specfile comment. - [!] Packager considers avoiding confusing `%foo_name` macros - [x] Libraries: The package name has a `python3-` prefix and uses the canonical project name - [-] Applications: A `python3-` prefix is not used - [x] The pyproject macros are used - [x] There are no bundled libraries - [x] The package complies with the Python and general Packaging Guidelines.
If you don't mind rust, it would be helpful if you could review the missing dependencies of maturin [1]. Otherwise, don't worry about it! [1] https://bugzilla.redhat.com/buglist.cgi?bug_id=2187698&bug_id_type=anddependson&bug_status=__open__&format=tvp
Thanks for the review. I dropped %fed_name. But I'm keeping %pypi_name. I actually picked that up from other Python package(r)s and I find it quite handy in referring to the upstream name, which quite often differs from the normalized name we use in Fedora. BR for pytest has been dropped, summary has been shortened, reference to upstream bug regarding missing tests has been added. > [-] Applications: A `python3-` prefix is not used Well, it's a bit of both. The package provides a standalone executable, but also a library. I believe Bottles in particular uses the library not the executable. Is it better then to name the package 'fvs' and create a python3-fvs subpackage for the library as suggested by https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_application_naming ? I'll wait for your feedback before providing updated links.
(In reply to Maxwell G from comment #2) > If you don't mind rust, it would be helpful if you could review the missing > dependencies of maturin [1]. Otherwise, don't worry about it! I'll have a look if there's something I feel confident reviewing. That's the least I can do.
I decided to rename the package to fvs and provide a library subpackage. Spec URL: https://download.copr.fedorainfracloud.org/results/gui1ty/reviews/fedora-rawhide-x86_64/05844321-fvs/fvs.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/gui1ty/reviews/fedora-rawhide-x86_64/05844321-fvs/fvs-0.3.4-1.fc39.src.rpm Copr build: https://copr.fedorainfracloud.org/coprs/gui1ty/reviews/build/5844321/
Copr build: https://copr.fedorainfracloud.org/coprs/build/5844400 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2187061-fvs/fedora-rawhide-x86_64/05844400-fvs/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
> I decided to rename the package to fvs and provide a library subpackage. I'm not a huge fan of this. One Python distribution really should map to one RPM package in most cases. If the project is primarily an application, its package should be named PROJECTNAME. If it's primarily a library, it should be named python3-PROJECTNAME. The python3-PROJECTNAME package may Provide PROJECTNAME if the project is primarily a library but also contains a secondary PROJECTNAME application/CLI. The PROJECTNAME application package may also Provide python3-PROJECTNAME if it contains a secondary PROJECTNAME library (you should use the `%py_provides` macro for this case; see https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_provides_and_requirements). The Python Naming Guidelines (https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_naming) provide more details. In this case, it seems `python3-FVS` (as a subpackage of an `python-FVS` SRPM) is the correct naming.
Thank you for the feedback. I think it boils down to what the primary purpose of this package is. Honestly, I don't feel confident in deciding that question. IIRC, it has been developed by one of the Bottles developers for a specific need in Bottles. Bottles uses the library. > In this case, it seems `python3-FVS` (as a subpackage of an `python-FVS` SRPM) is the correct naming. Well, the project name needs to be normalized. So, if we are going with a library package the names should be python-fvs and python3-fvs respectively. I know you have a ton more experience packaging than I do, but I still don't feel confident if this is primarily an app or a library. Looking at the project description - https://pypi.org/project/FVS/ - it appears to provide both: app and library. So, I'm inclined to go with the application naming advice: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_application_naming, in particular: "Consider adding a virtual provide according to Library naming above (e.g. python3-PROJECTNAME), if it would help users find the package." Should I ask for some more advice on the Python list perhaps? I definitely want to avoid having to rename the package later.
Nobody's going to make you rename the package later for this kind of thing. As long as you use one of the allowed naming schemes based on your best judgement, you're in the clear :). To me, it seems like this is mainly a library, and that's why you're packaging it. From what I can tell, this was purpose built for Bottles to use as a library. The CLI was added after the rest of the library code [1]. On the other hand, vkbasalt-cli is billed as a CLI application, so I'd name the package vkbassalt-cli and add `%py_provides vkbassalt`. > Well, the project name needs to be normalized. So, if we are going with a library package the names should be python-fvs and python3-fvs respectively. That's correct and what I meant to suggest! I'd create a python-fvs component with a python3-fvs subpackage. The python3-fvs subpackage can have `Provides: fvs = %{version}-%{release}`. If you prefer the other way around, that's fine with me as well. [1]: https://github.com/mirkobrombin/FVS/commit/b447058773e29fd7adab6d585ae08c32f4565e26
> That's correct and what I meant to suggest! I'd create a python-fvs > component with a python3-fvs subpackage. The python3-fvs subpackage can have > `Provides: fvs = %{version}-%{release}`. If you prefer the other way around, > that's fine with me as well. Okay. Let's treat it as a library. I renamed the package back to python-fvs and added a Provides for the executable. Spec URL: https://download.copr.fedorainfracloud.org/results/gui1ty/reviews/fedora-rawhide-x86_64/05847870-python-fvs/python-fvs.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/gui1ty/reviews/fedora-rawhide-x86_64/05847870-python-fvs/python-fvs-0.3.4-2.fc39.src.rpm Copr build: https://copr.fedorainfracloud.org/coprs/gui1ty/reviews/build/5847870/
Copr build: https://copr.fedorainfracloud.org/coprs/build/5847884 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2187061-python-fvs/fedora-rawhide-x86_64/05847884-python-fvs/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Maxwell's Python Package Review Template ============== Notes: (Please fix before importing) - [!] The `%description %_description` lines are not properly separated. For consistency, there should be 1-2 empty lines before and after these blocks just like there is e.g. between %build and %install. They're separate sections. - [!] For consistency, there should be two empty lines between %prep and %generate_buildrequires. %generate_buildrequires is a completely separate section, so it should be delaminated in the same way as %build and %install are. Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated - [x] The License tag reflects the package contents and uses the correct identifiers. - [x] The package builds successfully in mock. - [x] The package is installable (checked by fedora-review). - [x] There are no relevant rpmlint errors. - [-] The package runs tests in %check. Upstream does not provide unit tests, so %pyproject_check_import is used. - [x] The latest version is packaged or packaging an earlier version is justified. - [x] Packager considers avoiding confusing `%foo_name` macros. (Not a blocker) Packager considered and chose to keep %pypi_name. - [x] Libraries: The package name has a `python3-` prefix and uses the canonical project name - [-] Applications: A `python3-` prefix is not used Packager used the correct naming scheme for a library package. - [x] The pyproject macros are used - [x] There are no bundled libraries - [x] The package complies with the Python and general Packaging Guidelines. Package approved! On import, don't forget to add the package to release-monitoring.org and reference the review bug in the changelog/bodhi update. The release should start at 1 with a changelog entry / commit message similar to "Initial import. Closes rhbz#2187061." You can also give @python-packagers-sig `commit` ACLs if you wish. Thanks!
Thank you for the review! > Notes: > > (Please fix before importing) > > - [!] The `%description %_description` lines are not properly separated. For > consistency, there should be 1-2 empty lines before and after these blocks > just like there is e.g. between %build and %install. They're separate > sections. > - [!] For consistency, there should be two empty lines between %prep and > %generate_buildrequires. %generate_buildrequires is a completely separate > section, so it should be delaminated in the same way as %build and %install > are. I tidied up the spec file and put two empty lines between all sections.
The Pagure repository was created at https://src.fedoraproject.org/rpms/python-fvs
FEDORA-2023-53c2d12490 has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-53c2d12490
FEDORA-2023-53c2d12490 has been pushed to the Fedora 39 stable repository. If problem still persists, please make note of it in this bug report.