-
Notifications
You must be signed in to change notification settings - Fork 2k
[DOC] Fix broken discord links #4130
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
[DOC] Fix broken discord links #4130
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
.gitignore
Outdated
| testdata | ||
|
|
||
| # Python version | ||
| .python-version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should i remove?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
docs/docs.trychroma.com/package.json
Outdated
| "typescript": "latest" | ||
| } | ||
| }, | ||
| "packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should i remove?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 i wasn't sure what to do here because the docs just say to yarn install but yarn is no longer recommended as a global dependency, i ended up just using npm to build it.
docs/docs.trychroma.com/package.json
Outdated
| "typescript": "latest" | ||
| } | ||
| }, | ||
| "packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes please
.gitignore
Outdated
| testdata | ||
|
|
||
| # Python version | ||
| .python-version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes please
docs/docs.trychroma.com/README.md
Outdated
| - [ ] swag element in light/dark mode. eg the graphic on the index route | ||
| - [ ] turn on algolia once indexed | ||
| - [ ] turn back on "edit on github" button when public | ||
| - [x] turn back on "edit on github" button when public |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we keep the PR about discord links? a lot of the features todo are ood, so we'd want to refresh the whole thing at some point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah for sure... it was just one of those things i caught and wanted to quick-fix but it's better to keep the PR focused
## Description of changes This is just closing the loop on #4130 , which seems to have gotten out of sync with my local branch, so the revision requests I was responding to weren't getting pushed up, so I'll just close that one. Also, I noticed that in the meantime @jeffchuber already merged a PR #4286 with my changes, the only difference being that this adds a link to the channel. 🤷 So this is a really trivial PR, just making it so that I can close out the other one and maybe you like this minuscule enhancement. @jairad26 Co-authored-by: Really Him <hesereallyhim@proton.me>
|
This branch got borked. See #4363 |
## Description of changes This is just closing the loop on chroma-core#4130 , which seems to have gotten out of sync with my local branch, so the revision requests I was responding to weren't getting pushed up, so I'll just close that one. Also, I noticed that in the meantime @jeffchuber already merged a PR chroma-core#4286 with my changes, the only difference being that this adds a link to the channel. 🤷 So this is a really trivial PR, just making it so that I can close out the other one and maybe you like this minuscule enhancement. @jairad26 Co-authored-by: Really Him <hesereallyhim@proton.me>
Description of changes
Saw this Issue reporting a broken discord link and figured I'd put together a quick fix.
This link - https://discord.gg/rahcMUU5XV - still seems to work, but i think it's from a different owner/inviter. 🤷
Also, I added.python-versionto.gitignorebecause I like to use it for pyenv.Also, I checked off an item on the docs main README TODO's because I think it's completed.Also, when I ranyarnto install, and thenyarn devto run the docs, it made some auto-changes topackage.jsonandnext-env.d.ts. Yarn pins the package manager version to the package.json, which would explain the first change.I also noticed that it looks like the instructions are still using yarn 1 (aka yarn-classic), which is no longer really standard. yarn >=2 made a big change to a plug'n'play type system using
corepack. I would recommend we consider switching that over.Test plan
How are these changes tested?
I ran the docs site locally and verified the broken discord links.
pytestfor python,yarn testfor js,cargo testfor rustDocumentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs repository?