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

docs: generate list of all commands into readme #1313 #1321

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dirkbonhomme
Copy link

Pull Request check-list

Please make sure to review and check all of these items:

  • Does npm test pass with this change (including linting)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?

Description of change

Includes a list of all Redis commands into the README and provides an npm command to re-generate this list in case there's an update.

Done as a "fun" response to #1313 so no hard feelings if this doesn't get merged. The situation there was painfully recognizable though because my first experience using this library was by going to the Github homepage and searching for the command I wanted to use.

@stockholmux
Copy link
Contributor

stockholmux commented Mar 16, 2018

I had been thinking about this too. Great minds, eh?

I don't think, however, this is not the answer, execution wise. Not that SEO should be our goal but this looks like a keyword link dump and Google hates those. So, here are my thoughts:

  • We don't really need to link to each command.
  • I think a table might be more usable/readable. Perhaps 3 columns: command, supported (a green emoji check, which will be every row) and a third column for any notes, relevant for commands like HMSET.
  • The position on the page could be at the bottom right before the license/contributors. We could do an internal link at multiple parts inside the document.

As you might imagine this is not as simple as your solution, but I think it would yield good results.

@DanielSeehausen
Copy link

using this in some capacity (@stockholmux suggestion looks great) would be a solid improvement to the docs.

I imagine a lot of developers find themselves keyword searching for a package that implements a specific redis query. i.e., the only reason I found myself here is I was looking for the implementation of setrange

please don't let this die!

@dirkbonhomme
Copy link
Author

@DanielSeehausen Thanks for bringing this back to my attention, kinda forgot about it.

I've reviewed the tool based on @stockholmux' feedback:
https://github.com/NodeRedis/node_redis/blob/de18e0b9f941138b9ebd85f7920500337076e7a6/README.md#list-of-supported-commands

Some commands have notes with internal links to their explanation. Note sure if there are more commands that can or should have more explanation..

Any feedback?

@BridgeAR
Copy link
Contributor

BridgeAR commented Feb 5, 2019

This is a pretty huge list and takes a lot of space. I think moving this into a separate doc and adding a link to that would be best.

I feel that in general the main README should become smaller and not bigger. As soon as it's indeed small people will probably also find everything :-)

@DanielSeehausen
Copy link

@DanielSeehausen Thanks for bringing this back to my attention, kinda forgot about it.

I've reviewed the tool based on @stockholmux' feedback:
https://github.com/NodeRedis/node_redis/blob/de18e0b9f941138b9ebd85f7920500337076e7a6/README.md#list-of-supported-commands

Some commands have notes with internal links to their explanation. Note sure if there are more commands that can or should have more explanation..

Any feedback?

I think it looks great -- the external docs are a good idea as well

@DanielSeehausen
Copy link

@BridgeAR @dirkbonhomme wanted to ping to see if we were good with closing this, understanding external docs may be the way the wind is blowing!

@dirkbonhomme
Copy link
Author

@DanielSeehausen do you think it's useful if I update the tool to generate a new file COMMANDS.md (or other) and just add a link to that file from the README? Or abandon the idea altogether?

@DanielSeehausen
Copy link

I think that is a great idea @dirkbonhomme ! +1 from me

@dirkbonhomme
Copy link
Author

It's now generating a separate COMMANDS.md file instead of listing all supported commands in the README: https://github.com/dirkbonhomme/node_redis/blob/master/COMMANDS.md

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

Successfully merging this pull request may close these issues.

5 participants