Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 1448451 (qt5-doc) - Review Request: qt5-doc - Qt5 - Complete documentation
Summary: Review Request: qt5-doc - Qt5 - Complete documentation
Keywords:
Status: CLOSED RAWHIDE
Alias: qt5-doc
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: qt-reviews
TreeView+ depends on / blocked
 
Reported: 2017-05-05 12:43 UTC by Helio Chissini de Castro
Modified: 2017-11-20 19:06 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-11-20 19:06:40 UTC
rdieter: fedora-review+


Attachments (Terms of Use)

Comment 1 Kevin Kofler 2017-05-05 22:32:30 UTC
Thanks for trying this out and showing that it can be done, but I do not agree with this approach at all.

From a user perspective, even if you want to build qt5-doc from a single SRPM, you still have to split it into subpackages, possibly with a metapackage that requires them all. What's the point of splitting qt5-* including -devel if -doc is monolithic?

And doing things this way is an issue for QtWebEngine because QtWebEngine can be upgraded without the rest of Qt (as has been done for 5.8), and so the docs in this monolithic package (or even in a subpackage of it if you introduce them) will not match the actual package. Therefore, in my role of the QtWebEngine maintainer, I explicitly disapprove the Obsoletes on qt5-qtwebengine-doc.

Comment 2 Kevin Kofler 2017-05-05 22:37:07 UTC
(And as I wrote on IRC, I also disagree with this approach on technical grounds, because you are no longer building the documentation from source in the specfile, it has to be manually prepared (in a process that takes hours) before even generating the SRPM.)

Comment 3 Rex Dieter 2017-05-08 18:01:37 UTC
I can review this

Comment 4 Rex Dieter 2017-05-08 18:26:04 UTC
re: comment #1

I agree with Kevin in general.

1. Ideally we'll want to create a bunch of subpkgs + umbrella metapackage instead of this initial monolithic approach.  That said, I won't consider that a review blocker, and we can implement that after import.

2. In the case of qtwebengine, when/if we have to, we can omit its docs from here as needed.

Comment 5 Helio Chissini de Castro 2017-05-08 19:28:29 UTC
The split can be done now, but then will end up in subpackages that will be broken as well.

To mention, for example, let's say that i will install qt5-doc-qtdeclarative, and inside it we have links for doc from qtbase, qtxmlpatterns, etc, etc..
So we will need to require this packages on the subpackage.

Unless we create a script to parse all documentation to discover all the requires, this will be very difficult to track.

The solution of have the parent meta package solve this, but then, if someone decide to install one single module, we will end up in the same situation as before.

I can't understand why this is a big deal, because i don't see anyone complaining on old package qt4 doc that is big and monolithic as well.

Comment 6 Kevin Kofler 2017-05-08 20:27:45 UTC
The whole Qt 4 was monolithic, so of course the -doc package was, too.

And I don't see the big deal if a link to, e.g., QtCore is broken if you don't have QtCore docs installed, that's IMHO completely expected.

Comment 8 Rex Dieter 2017-05-09 13:41:47 UTC
Thanks.
Release: 
Naming: ok

License: ok

Sources: ok (generating script included).

macros: ok
1.  SHOULD use %{_qt5_docdir} instead of %{_docdir} macro, ie, replace %{_docdir}/qt5/ references with just:  %{_qt5_docdir}

2.  subpackage deps SHOULD use full version-release, like:
Requires: qt5-qtbase-doc = %{version}-%{release}

scriplets: n/a

3.  SHOULD use proper Release tag, something like
Release: 0.3.beta3%{?dist}
instead of existing
Release: 0.beta.3%{?dist}
style

I won't consider these blockers, and can easily be fixed after import.


APPROVED

Comment 9 Gwyn Ciesla 2017-05-10 14:55:52 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/qt5-doc

Comment 10 Rex Dieter 2017-11-20 19:06:40 UTC
This was imported long ago, closing


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