Skip to content

Fix/man page#549

Closed
anadahz wants to merge 4 commits intomasterfrom
fix/man-page
Closed

Fix/man page#549
anadahz wants to merge 4 commits intomasterfrom
fix/man-page

Conversation

@anadahz
Copy link
Copy Markdown

@anadahz anadahz commented Jun 17, 2016

anadahz added 2 commits June 17, 2016 18:43
Update ooniprobe man page to be relevant to the Debian package version.
* Cleanup unneeded parts of installation and developer's documentation.
* Add missing command line options
* Add missing tests
* Fix old URLs and PATHs
Comment thread data/ooniprobe.1
Display the ooniprobe version and exit.
.TP
.BR \-\^Q " or " \-\-queue
AMQP Queue URL amqp://user:pass@host:port/vhost/queue
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is an 'AMQP'? This needs a better description.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In sort this is used to make ooniprobe work with blocked: openrightsgroup/cmp-issues#78
Not sure how we can describe this in a way that makes senses to the user.

@coveralls
Copy link
Copy Markdown

coveralls commented Jun 17, 2016

Coverage Status

Coverage decreased (-0.07%) to 72.013% when pulling 13113c3 on fix/man-page into 358a583 on master.

@willscott
Copy link
Copy Markdown

These look good, but there are remaining nits from the ticket in trac that aren't addressed by this, like reference to missing mlab test decks among others. Is this meant to be a combined pull to fix everything?

@anadahz
Copy link
Copy Markdown
Author

anadahz commented Jun 17, 2016

@willscott Do you think that mlab test decks are relevant to the ooniprobe user?

@anadahz
Copy link
Copy Markdown
Author

anadahz commented Jun 17, 2016

It will be nice to have an auto generated man page with the actual ooniprobe version and date.

@willscott
Copy link
Copy Markdown

I think they aren't relevant, which is why having them referenced in the man page is potentially confusing.

The queue thing needs at least sufficient documentation for someone technical to figure out what's going on. even that github bug leaves it pretty unclear what's going on here. Is this some alternative way to get test input (that's stored in a rabbitmq server somewhere?)

Saying, 'there's this option that causes a set of acronyms to happen' seems like it can only increase confusion.

@coveralls
Copy link
Copy Markdown

coveralls commented Jun 17, 2016

Coverage Status

Coverage decreased (-0.07%) to 72.013% when pulling 45cb387 on fix/man-page into 358a583 on master.

@anadahz
Copy link
Copy Markdown
Author

anadahz commented Jun 17, 2016

Removed the mlab deck references.
I agree the AMQP option introduces confusion.
I'm not aware of a way to get a test input.
Perhaps @dantheta can provide as with some useful test input?

@coveralls
Copy link
Copy Markdown

coveralls commented Jun 17, 2016

Coverage Status

Coverage remained the same at 72.086% when pulling 2f819fe on fix/man-page into 358a583 on master.

@dantheta
Copy link
Copy Markdown
Contributor

I don't think there is a way to get test input for the AMQP routines - it's intended to point at an AMQP (Rabbit) server. However, the AMQP options are only useful on the bin/ooniprobequeue command (which uses a variant director), not bin/ooniprobe; I guess the command line options appear for ooniprobe because the command-line options are in shared code.

It might be possible to wrap the parseOptions function so that it uses an extended Options class for the DaemonDirector. That way the --amqp option wouldn't appear when someone was running bin/ooniprobe --help, only when they were running bin/ooniprobequeue.

@willscott
Copy link
Copy Markdown

an initial set of additional changes I'll propose: willscott@aab516c

anadahz referenced this pull request in willscott/ooni-probe Jun 29, 2016
@anadahz
Copy link
Copy Markdown
Author

anadahz commented Jun 29, 2016

@willscott apart from the queue comments (willscott@aab516c#commitcomment-18060250) looks good to me.

@willscott willscott mentioned this pull request Jun 29, 2016
@willscott
Copy link
Copy Markdown

Re-added AMQP help line to my branch ooni/probe-legacy@fix/man-page...willscott:fix/man-page

And filed bug to do it the right way as suggested https://github.com/TheTorProject/ooni-probe/issues/560

Merge/squash my branch into this pull request?

@hellais
Copy link
Copy Markdown
Member

hellais commented Jun 29, 2016

@dantheta actually since ooniprobe 1.4.0 (ooni/probe-legacy@1192ca0 specifically) there is no longer the need to use the specialised ooniprobequeue command to use the messagequeue, but it will invoke that code path upon launching it with the --queue option.

You should probably keep this in mind when you update to a later version of ooniprobe for ORG.

@dantheta
Copy link
Copy Markdown
Contributor

Ah, that's really cool. Thanks!

@willscott
Copy link
Copy Markdown

Lets also try to improve the documentation of the -p option, which is unclear (https://trac.torproject.org/projects/tor/ticket/11988)

@hellais
Copy link
Copy Markdown
Member

hellais commented Nov 22, 2016

I believe this is now superseeded by the manpage docs being generated from markdown files. I believe all the feedback in here should be integrated in there, but please double check to be sure.

@hellais
Copy link
Copy Markdown
Member

hellais commented Jan 9, 2017

Can I close this?

@anadahz
Copy link
Copy Markdown
Author

anadahz commented Jan 12, 2017

There is significant amount of documentation missing from the markdown docs that should either be added to the docs itself or referenced to the manpages as links or external files (eg: in /usr/share/doc).

The documentation topics that are not covered by the current markdown manual pages are:

  • Default collector, bouncer addresses
  • Test enumeration and description
  • Default paths for reports, decks and config files
  • OONI terms explanation (tests, decks, test helper,...)
  • Configuring ooniprobe
  • Running decks
  • Running net tests
  • Optional packages (obfs4proxy)
  • Bridges and obfsproxy bridges
  • Third party tests and dependencies

@hellais
Copy link
Copy Markdown
Member

hellais commented Jan 23, 2017

There is significant amount of documentation missing from the markdown docs that should either be added to the docs itself or referenced to the manpages as links or external files (eg: in /usr/share/doc).

Sounds good. Can you add these to the markdown files as part of a new PR? I also don't think all of these need to necessarily be present inside of the manpage as they are documented elsewhere.

@hellais hellais closed this Jan 23, 2017
@anadahz
Copy link
Copy Markdown
Author

anadahz commented Jan 23, 2017

@hellais Sure, though if this issue is closed it will be most probably forgotten.

I also don't think all of these need to necessarily be present inside of the manpage as they are documented elsewhere.

Which places are you referring to?
It makes sense to add a URL in the man pages pointing to the documentation already mentioned elsewhere.

@hellais
Copy link
Copy Markdown
Member

hellais commented Jan 23, 2017

Which places are you referring to?

The README has most of these things.

It makes sense to add a URL in the man pages pointing to the documentation already mentioned elsewhere.

Sure go for it!

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.

5 participants