search

coniks-sys / coniks-go

A CONIKS implementation in Golang
http://coniks.org
Other
116 stars 30 forks source link

Fix godoc display issues, other doc fixes #132

Closed vqhuy closed 7 years ago

masomel commented 7 years ago

@arlolra @Liamsi Should we add documentation to the executables' main package? Original discussion: https://github.com/coniks-sys/coniks-go/pull/131#discussion_r89150645

vqhuy commented 7 years ago

@masomel : I rebased this branch with master and moved some subpackages to /internal in bdcf27b6fe1674eee244bd0c75b8b103aaba3cb4. What do you think? godoc will not show bar package if it is an internal package of foo (foo/internal/bar).

You can use git fetch; git reset --soft origin/fix-doc in your local branch to avoid conflict.

liamsi commented 7 years ago

@arlolra @Liamsi Should we add documentation to the executables' main package? Original discussion: #131 (comment)

I agree with @c633 that this isn't really necessary.

arlolra commented 7 years ago

Is this pull meant to still be in WIP? If so, what's left to do? It seems pretty reasonable to me.

masomel commented 7 years ago

Is this pull meant to still be in WIP? If so, what's left to do?

I'm doing one last pass over all the documentation to make sure we didn't miss anything. I don't think it needs to be in WIP for that though.

EDIT: I should only need 1-2 more hrs for this; any changes should be minor.

masomel commented 7 years ago

So, I know that we had talked about not needing summaries for executable commands, but their main packages still appear in godoc, and I think it's awkward that these packages don't have a summary. What if the summary just said something like "Executable X. See README for usage information"?

arlolra commented 7 years ago

What if the summary just said something like "Executable X. See README for usage information"?

Sounds reasonable.

arlolra commented 7 years ago

FYI, I fixed up the go formatting issues and rebased.

vqhuy commented 7 years ago

LGTM. Thank you @masomel @arlolra

masomel commented 7 years ago

Is this ready to be merged?

vqhuy commented 7 years ago

From my side, yes.