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.

Sign up for GitHub

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

Jump to bottom

Rework readme #198

Merged
merged 7 commits into from
Oct 10, 2019
Merged

Rework readme #198

merged 7 commits into from
Oct 10, 2019

Conversation

JacobPlaster
Copy link
Contributor

Description:

Cleans up readme and adds new doc files

Breaking changes:

None

New features:

  • docs/rest_v2.md
  • docs/v1.md
  • docs/ws_v2.md
  • New readme format
  • Tonnes of new function comments

Fixes:

None

PR status:

  • Version bumped - not required
  • Change-log updated - not required

f3rno
f3rno suggested changes Oct 8, 2019
View reviewed changes
Copy link

@f3rno f3rno left a comment

Choose a reason for hiding this comment

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

I think we definitely need a list of all functions/data types at the top of each API doc file, similar to JSDoc. Is there any way this can be done via the template system godocdown provides? If not, we could potentially parse the generated files for #### func .... and #### type .... lines, grab the description below the 'go...' block, and pre-pend the list as a post-doc-build step. Otherwise it is not obvious what the library provides unless one scans the entire file.

README.md Outdated
* Connection multiplexing
* Types for all data schemas

## Instillation
Copy link

Choose a reason for hiding this comment

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

Typo & extra newline below

docs/rest_v2.md
#### func (*Client) NewAuthenticatedRequestWithBytes

```go
func (c *Client) NewAuthenticatedRequestWithBytes(permissionType bitfinex.PermissionType, refURL string, data []byte) (Request, error)
Copy link

Choose a reason for hiding this comment

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

I leave it up to you, but you may want to consider making this and a few other functions private, excluding them from the docs.

@JacobPlaster
Copy link
Contributor Author

Otherwise it is not obvious what the library provides unless one scans the entire file.

Yeah completely agree, I really dont want to make this a manual process though. Hopefully there is a nice python package for parsing markdown that I can use

f3rno
f3rno approved these changes Oct 10, 2019
View reviewed changes
Copy link

@f3rno f3rno left a comment

Choose a reason for hiding this comment

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

lgtm 👍

You should create an issue for pre-pending a ToC to the API docs with a script later if merged like this.

@JacobPlaster
Copy link
Contributor Author

Created issue #199

@JacobPlaster JacobPlaster merged commit e5ec55f into bitfinexcom:master Oct 10, 2019
@JacobPlaster JacobPlaster deleted the rework-readme branch October 10, 2019 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@f3rno f3rno f3rno approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@JacobPlaster @f3rno