Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

begin asciidoc api man pages #273

Merged
merged 11 commits into from
Jul 18, 2015
Merged

begin asciidoc api man pages #273

merged 11 commits into from
Jul 18, 2015

Conversation

garlick
Copy link
Member

@garlick garlick commented Jul 17, 2015

This PR adds documentation for most of the functions in src/common/libflux/handle.h.

Here are the misdemeanors in this PR that I am aware of:

  • I forked the dictionary from doc/cmd into doc/api. Probably the two files should be merged.
  • COPYRIGHT.adoc is also forked without change. I couldn't figure out how to affect the asciidoc include path on the command line of a2x.
  • api pages are not wired into flux help. Should they be?
  • spellcheck script is run from $(top_srcdir)/doc/cmd/spellcheck. Should that go with a shared dictionary in doc/etc or something like that?

@grondo
Copy link
Contributor

grondo commented Jul 17, 2015

Wow, a lot of work here. Nice.

My opinion is that API manpages should not be wired to flux help That is for commands. However, it would be nice to have a way to view manuals for the API from within a build directory, so I can see why that would be nice.

Good ideas on moving spellcheck around. My suggestion would be to put the script and dictionary under test somewhere since it is a testing only thing. But really I'd be fine with your suggestion too

@garlick
Copy link
Member Author

garlick commented Jul 17, 2015

Oh test makes good sense.

What about a new subcommand like flux man [section] topic? Seems like it would be nice to be able to read formatted man pages when working with the source on a system that flux isn't installed on.

I just remembered that man wants a man3 subdir. I have a hack to symlink doc/cmd to doc/man1. Should we just make the canonical directories man1 and man3?

@grondo
Copy link
Contributor

grondo commented Jul 17, 2015

We may indeed have an intermittent hang in wreck tests. First build hung for this PR -- I restarted and seems to be ok now

@garlick
Copy link
Member Author

garlick commented Jul 17, 2015

Maybe the default grace period we just cranked down is too short?

@grondo
Copy link
Contributor

grondo commented Jul 17, 2015

Could something that exports appropriate MANDIR in a working dir be used instead of flux man? Maybe flux start could do this automatically?

I'm fine with flux man actually, but it might feel like a silly command for an installed flux?

(That being said, knowing you are getting the man pages associated with the flux command you are running would be a real nice property. Also, maybe you could expand the command to serve up html pages too

flux man --html --port=8080

@garlick
Copy link
Member Author

garlick commented Jul 17, 2015

OK, I moved the dictionary and spellcheck scripts as discussed, and changed the directories to man1 and man3. It turns out that flux help now finds the API man pages without my doing anything, so if that works, perhaps we can call it good.

@grondo
Copy link
Contributor

grondo commented Jul 17, 2015

Can we squash f848139 into b7ba54c?

Ideally we wouldn't be adding a bunch of new files then moving them all in a single PR, but I can see that will be a major PITA to fixup and this is probably one of the cases where it isn't worth it.

@garlick
Copy link
Member Author

garlick commented Jul 17, 2015

It only took 20m or so to rebase and fix all of it.

@grondo
Copy link
Contributor

grondo commented Jul 18, 2015

Oh, man. I feel really bad about that. :person_frowning:

I will say that this PR is now an example and a shining beacon that will inform the behavior of pull requests for a long time to come. I am showing it to my children now.

I shall press the merge button with pleasure!

grondo added a commit that referenced this pull request Jul 18, 2015
begin asciidoc api man pages
@grondo grondo merged commit 2826d0b into flux-framework:master Jul 18, 2015
@grondo grondo removed the review label Jul 18, 2015
@garlick garlick deleted the apidocs branch July 20, 2015 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants