https://bugzilla.redhat.com/show_bug.cgi?id=1914689 Richard Shaw <hobbes1069@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Flags|needinfo?(hobbes1069@gmail. | |com) | --- Comment #11 from Richard Shaw <hobbes1069@xxxxxxxxx> --- (In reply to code from comment #10) > > Issues: > ======= > - It is a lot easier to review if you update both the spec and SRPM. This > will > be mandatory for approval, since the SRPM is needed to create the initial > dist-git repo. (Even better, bump the release when you make changes.) Sorry the intent was to get a quick re-review from Miro as I essentially rewrote the specfile based on his feedback. > - The LICENSE.md file for the bundled semantic-ui must be installed with > %license too. > > - There is a bundled copy of semantic-ui included. You must follow > https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling. Since > upstream does not *technically* support building with an external copy, you > have the option to publicly contact upstream about a path to doing so, and > then add the appropriate virtual Provides: > > Provides: bundled(js-semantic-ui) = 2.4.1 > > However, all it would take to use a separately-packaged copy would be to > replace this directory with a symbolic link. Then you could package > js-semantic-ui as a dependency in accordance with > https://docs.fedoraproject.org/en-US/packaging-guidelines/JavaScript/ and > https://docs.fedoraproject.org/en-US/packaging-guidelines/Web_Assets/. This > has advantages: > > https://fedoraproject.org/wiki/ > Bundled_Libraries#Why_Bundled_Libraries_are_a_problem. > > Note that if you do package or bundle js-semantic-ui, you must > compile/minify > the CSS and JS yourself from the original sources in the RPM build process, > and you must include the original sources in the RPM, not just the minified > ones. > > …but note the next issue, below! > > - A new major version, 2.0.0, was released in late January; you should > package > it instead. This affects the semantic-ui situation: “JS/CSS assets are now > copied across by sphinx when builing, rather than being copied by the > extension”. From testing building sphinx-tabs’s own documentation, I don’t > see any reliance on semantic-ui at all. Maybe the problem has gone away. > You > should look into it more closely than I did. There's not longer anything bundled which is good because I only need this as a dep for OpenColorIO 2.0. I don't need any more packages :) > - You correctly noted in %check that Sphinx is too old for this package on > EPEL > (both 7 and 8). Besides, the pyproject-rpm-macros are not available on > EPEL. > So, remove the EPEL-specific cruft from the spec file. > > Each “%{python3_pkgversion}” should just become a “3”. Done. > This > %{?python_provide:%python_provide python3-%{pypi_name}} > should be deleted; even if you were packaging for EPEL, you should > conditionalize it because this macro should not be used on Fedora; see > > This was never needed even for EPEL, as both EPEL and Fedora define the > macro: > %{?!python3_pkgversion:%global python3_pkgversion 3} I cleaned it up. I only plan to build for Fedora. I only need it for Rawhide but I may build it for f33 as well. > BR’s should be written like: > > BuildRequires: python3-devel > BuildRequires: python3dist(setuptools) Done. > - “%pyproject_save_files sphinx_tabs” could become > “%pyproject_save_files %{python_module_name}”, if you like Done. > - Instead of manually specifying BR’s for testing, change > %pyproject_buildrequires > to > %pyproject_buildrequires -x testing > > You can fix the missing python3dist(bs4) by replacing "bs4" with > "beautifulsoup4" in setup.py; see https://pypi.org/project/bs4/. > > You can loosen the pytest version restriction, which is currently <4; > version > 6 does work. > > Unfortunately, you still cannot run the tests without > python3dist(pytest-regressions), so choose one of the following: > > - Package python-pytest-regressions. > - Figure out how to patch it out of the tests, and run them. > - Go back to just “%pyproject_buildrequires”, and put a comment where the > %check section would be explaining the missing dependency. > > If you do get the tests working, > %check > %pytest > should work just fine. Again, since this is only being used as a build dep for me and I don't need anymore packages I think I'll leave it alone for now. > - It would be nice if you build the HTML documentation and installed it in a > -doc subpackage. Just set PYTHONPATH and use sphinx-build. Done. -- 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 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
