Nice!

The HTML documentation looks very useful.

A couple comments for the man pages:

• there is no page for sample-rpkg itself. Will this be maintained manually?
• at first I was confused that the header and Name section use a dash between command and subcommand (e.g. sample-rpkg-build). However the synopsis shows it correctly as a subcommand. This is not really an issue, man git-clone shows the exact same thing.

A further improvement might be better handling of aliases. Duplicating the same man page for ci and commit makes little sense to me, it would be nicer to have ci redirect users to documentation of commit. Currently only man page mentions the alias, HTML version does not have this info.

On Fri, 2018-01-05 at 10:29 +0000, Lubomír Sedlář wrote:

lsedlar commented on the pull-request: Build documentation by sphinx that you are following:
``
Nice!

The HTML documentation looks very useful.

A couple comments for the man pages:

• there is no page for sample-rpkg itself. Will this be maintained
  manually?

This should be handled automatically as much as possible. I didn't add
it because I thought rpkg now does not deliver an executable command
named rpkg (to avoid potential confusion if rpkg-client wants to
generate documents as well).

But, seems it would be useful to generate a page for sample-rpkg and
it could also be reused by downstream client tools, e.g. fedpkg, to
generate such a page for themselves.

Keep this PR opening. I'll fix this in another commit and push to this
PR.

• at first I was confused that the header and Name section use a
  dash between command and subcommand (e.g. sample-rpkg-build).
  However the synopsis shows it correctly as a subcommand. This is not
  really an issue, man git-clone shows the exact same thing.

A further improvement might be better handling of aliases.
Duplicating the same man page for ci and commit makes little
sense to me, it would be nicer to have ci redirect users to
documentation of commit. Currently only man page mentions the
alias, HTML version does not have this info.
``

Yeah, make sense to me as well to handle the alias particularly. I'll
file an issue for this point.

Thanks for your review. :)

Rebased and fixed conflicts. Both HTML document and manpage are generated for sample-rpkg now.

Updated demo docs:

HTML: https://cqi.fedorapeople.org/rpkg-demo-docs/html/
manpages: https://cqi.fedorapeople.org/rpkg-demo-docs/man/

Ready for review again.

Patch is rebased and updated with some minor changes in order to work with latest code. In addition, sample-rpkg.rst is removed from source/commands/ directory as there is already cli.rst generated.

I'm back to this patch and going to merge it, because some subcommands have been added to rpkg in the past and there will be more and existing commands will be enhanced, e.g. to support build from stream branches. By merging this patch, it is possible to provide a good documentation to users and easy for us to write release notes for future versions than the simple plain text file CHANGELOG.rst.

CHANGELOG.rst will not be removed IMO, it could be there to include a simple list of changes, but release notes in doc/ could provide more info and details, e.g. deprecations.

Do you plan to build the documentation as part of RPM build? Using python3 could be an issue then.

When I try to create the documentation in a fresh clone, I'm getting an error:

$ LANG=C make html
rm: cannot remove 'source/cli.rst': No such file or directory
make: *** [Makefile:16: clean-files] Error 1

The output that you linked looks good to me.

Undefined name templates.

This redefines the parser argument. Is that really correct?

Pretty please pagure-ci rebuild

Do you plan to build the documentation as part of RPM build? Using python3 could be an issue then.

For current stage, we could just push documentation to docs.pagure.org/rpkg.

I'm not sure if it makes sense to packagers to ship docs within RPM. If it does and someone wants it, then we can do that, and I thought it probably could be built only for and since f29.

I'm not sure if it makes sense to packagers to ship docs within RPM. If it does and someone wants it, then we can do that, and I thought it probably could be built only for and since f29.

It would probably work for all Fedoras, they all have Python 3. I was concerned about internal builds, but I agree that they really don't need the documentation.

@lsedlar both of the issues are fixed.

@lsedlar any comment to this PR?

Nothing else. Looks good to me. :100:

Commit 38727bba fixes this pull-request