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

README rework #4209

Merged
merged 12 commits into from
Nov 15, 2023
Merged

README rework #4209

merged 12 commits into from
Nov 15, 2023

Conversation

jjbrosnan
Copy link
Contributor

This PR reworks our README in the root directory of deephaven-core. It uses Matt's PR as a launch point. Some comments from that PR can't currently be applied:

  • There are no docs for R. The only documentation comes from the ? operator in R.
  • I don't link directly to the how to launch and build from source guide because it's out of date. I've made a PR to update that document, and plan on doing that soon. When that's done, I'll link to it directly.
  • I couldn't think of a great way to link to some of our most important docs. I'll likely come up with a good solution soon.

@jjbrosnan jjbrosnan added documentation Improvements or additions to documentation NoDocumentationNeeded labels Jul 20, 2023
@jjbrosnan jjbrosnan added this to the July 2023 milestone Jul 20, 2023
@jjbrosnan jjbrosnan self-assigned this Jul 20, 2023
@jjbrosnan jjbrosnan added the NoReleaseNotesNeeded No release notes are needed. label Jul 20, 2023
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@elijahpetty elijahpetty left a comment

Choose a reason for hiding this comment

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

Not sure if a README is classified as a how-to guide or a reference doc, so I'm not sure if the tone matches the style guide. No significant comments, though.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@mattrunyon
Copy link
Contributor

The info I had put in my PR is more contributing info and how to work in the codebase. This does trim down some of the main readme, but IMO there's still info that doesn't belong like monitoring docker logs or maybe even the PSK info.

Just my opinion

  • README/GitHub is for developers who will be editing code in the repo. The start of the readme should kick users to the docs site if they just want to install and run Deephaven
  • Docs site is for using Deephaven the product. Some developer info like creating plugins might belong there

Having user info in the readme just doubles maintenance. If they are out of sync it could also create some confusion. Is the repo readme the bleeding edge docs, or is it out of date?

Just for an example, look at the Tensorflow or ScikitLearn readmes. They have a blurb about what the repo is, a very basic install (in their case it's just a pip install line) and then link out to detailed instructions and tutorials. Then development/contribution info.

@mofojed mofojed mentioned this pull request Jul 24, 2023
Copy link
Member

@devinrsmith devinrsmith 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 have a lot of issues w/ the related docs. Not necessarily something to tackle w/ this PR, but I think there's a lot of follow-up we should do.

I'm wondering if all the building and required dependency details should be moved to BUILD.md or DEVELOPMENT.md doc? We can link to it from here. I imagine the ideal readme would simply provide a single docker command to get up and running.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@mattrunyon
Copy link
Contributor

I still think there is too much duplicated content between the readme and docs site. IMO there should be no duplicated content between the two and the readme should hard push users to the docs site where the start guides are maintained. I think any actual content here should be unique from the docs site. No reason to have duplicates of the guides which we'll forget to update again in this readme

README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@margaretkennedy margaretkennedy left a comment

Choose a reason for hiding this comment

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

Can we keep this README as bare bones as possible and point users to the docs site? The benefit of that is the maintenance burden falls on one page instead of two.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@jjbrosnan jjbrosnan merged commit 81a90cb into deephaven:main Nov 15, 2023
10 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation NoDocumentationNeeded NoReleaseNotesNeeded No release notes are needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants