fix: correct clone URL in README Quickstart#198
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the README.md by correcting the repository URL and directory names to XortexAI/XMem, standardizing markdown emphasis syntax, and improving whitespace formatting. Review feedback identifies a missing directory change command in the Chrome Extension installation section that could lead to user errors and notes an inconsistency in the FALLBACK_ORDER configuration format compared to other documentation sections.
| git clone https://github.com/XortexLabs/xmem.git | ||
| cd xmem | ||
| git clone https://github.com/XortexAI/XMem.git | ||
| cd XMem |
There was a problem hiding this comment.
While updating the directory name to XMem here is correct, the 'Install the Chrome Extension' section (lines 331-333) is also missing a cd xmem-extension command after cloning. Users following these steps sequentially will still be inside the XMem directory, which will cause the npm install command in the next step to fail. It would be best to add the missing cd command to that section as well to ensure the Quickstart remains functional.
|
|
||
| > [!TIP] | ||
| > For a fully local setup with no cloud dependencies: | ||
| > |
There was a problem hiding this comment.
The example value for FALLBACK_ORDER in the tip ('["ollama"]') uses a JSON list format, which is inconsistent with the comma-separated format shown in the configuration table at line 370. While Pydantic-settings can often parse JSON strings for list types, using a simple comma-separated string (e.g., FALLBACK_ORDER=ollama) is more standard for .env files and matches the documentation elsewhere in this file. This inconsistency might lead to validation errors if the string is not parsed as expected by the validate_fallback_order check in src/config/settings.py.
|
| Filename | Overview |
|---|---|
| README.md | Formatting-only changes (blank lines, italic syntax, blockquote spacing) merged in; the clone URL and cd directory fixes described in the PR are absent from the diff |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[README.md diff] --> B{Change type}
B --> C[Blank line removed lines 11-12]
B --> D[Italic syntax *text* to _text_ lines 136 140]
B --> E[Blank line added before benchmark section line 306]
B --> F[Empty blockquote lines added inside TIP callout lines 371-374]
B --> G{Claimed in PR description}
G --> H[git clone URL fix XortexLabs to XortexAI - Not in diff]
G --> I[cd xmem to cd XMem - Not in diff - Line 316 still says cd xmem]
Comments Outside Diff (1)
-
README.md, line 314-318 (link)Stated fix not applied —
cd xmemstill presentThe PR description says it changed
cd xmem→cd XMemto match the actual cloned directory name, but the diff does not contain this change and line 316 still readscd xmem. On case-sensitive filesystems (Linux), ifnpx create-xmem@latestcreates a directory namedXMem, thecd xmemcommand will fail with "No such file or directory". The PR description also mentions fixing agit cloneURL (XortexLabs/xmem.git→XortexAI/XMem.git), but there is nogit clonecommand anywhere in the Quickstart section — so that fix either targets the wrong section or was never applied.
Reviews (2): Last reviewed commit: "Merge branch 'main' into fix/readme-clon..." | Re-trigger Greptile
|
Thank you for the contribution @nihalsheikh |
|
@nihalsheikh please fix the merge conflicts |
|
Hey @ishaanxgupta I've resolved the merge conflicts and accepted the incoming changes from |
|
I think the changes are fixed as I dont see updation of commands in your PR |
|
When resolving the merge conflicts, I accepted the incoming changes from |
Summary
XortexLabs/xmem.git→XortexAI/XMem.gitcd xmem→cd XMemto match the actual cloned directory nameCloses #
Testing
Notes
Closes #197