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

Fix up source code comments #184

Closed
bkircher opened this issue Feb 18, 2021 · 10 comments
Closed

Fix up source code comments #184

bkircher opened this issue Feb 18, 2021 · 10 comments
Assignees
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@bkircher
Copy link
Contributor

Cool thing is that gsclient-go documentation coverage is quiet good. But there are some corners that could need some polishing.

Go through package and docs for public types and functions in gsclient-go and follow suggestions in this article. Also make sure every package has a dedicated doc string.

image

@bkircher bkircher added the documentation Improvements or additions to documentation label Feb 18, 2021
@bkircher bkircher added this to the v3.4.1 milestone Feb 19, 2021
@nvthongswansea
Copy link
Contributor

@bkircher Could you give an example of comment that needs to be polished, please?

@bkircher
Copy link
Contributor Author

bkircher commented Mar 5, 2021

If you run godoc locally and compare the docs with what is in the, say, standard libs you see quiet a few differences. I think all is outlined in the article linked above.

@nvthongswansea
Copy link
Contributor

nvthongswansea commented Mar 11, 2021

@bkircher I've spent hours to get this godoc server running locally. Luckily, it runs perfectly at last. The doc of gsclient-go looks ok to me
image
Remember: the package name is github.com/gridscale/gsclient-go/v3 not github.com/gridscale/gsclient-go, since it uses gomod already.

@nvthongswansea
Copy link
Contributor

nvthongswansea commented Mar 11, 2021

@bkircher FYI, official docs site is docs. For comparison: gsclient-go and k8s client-go

@bkircher
Copy link
Contributor Author

I've spent hours to get this godoc server running locally.

It's just typing godoc and opening http://localhost:6060/pkg/, really. 🙂

@nvthongswansea
Copy link
Contributor

I've spent hours to get this godoc server running locally.

It's just typing godoc and opening http://localhost:6060/pkg/, really. 🙂

Didn't work on my machine. Issue link golang/go#15049

@nvthongswansea
Copy link
Contributor

The command that finally works is godoc -goroot /usr/share/go.

bkircher added a commit that referenced this issue Mar 11, 2021
Let comment lines begin with a space. Use sentences to let godoc work
it's magic.

See issue #184.
bkircher added a commit that referenced this issue Mar 11, 2021
@bkircher
Copy link
Contributor Author

Ah understand 😄

I added two commits that add a package description (rendered in godoc Overview section) and fix up a bit of line wrapping.

Anyways, this here is not only about the representation of docs (and that it integrates nicely with Golang tooling) but also about the content. Take

http://localhost:6060/pkg/github.com/gridscale/gsclient-go/v3/#StorageCreateRequest

for example (if you start your local godoc). StorageType can be nil but docs don't tell me (the user of the library) what storage type is actually chosen if I omit this value. (:joy: To be fair, API docs are also very silent about this.)

I think we could improve here a bit over time by giving the programmer more insightful, useful documentation on what actually happens in API calls like the above.

But let's prepare a release. Can't wait to get that sped improvement into gscloud!

@bkircher
Copy link
Contributor Author

Another good example of what could be improved are docs like this:
https://github.com/gridscale/gsclient-go/blob/master/template.go#L41

// TemplateProperties JSOn struct of properties of a template
type TemplateProperties struct {

Programmers know that this is marshaled to JSON by looking at the struct. Also this is not really important. A better comment would include

  • what this struct represents
  • what it is used for
  • or how to use it

@bkircher
Copy link
Contributor Author

I fixed above in 538abc3. Take a look. Also fixed the rest of issues in templates.go

You want to go through the other objects and fix the worst? @nvthongswansea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants