docs: improve README with setup instructions - #288
Conversation
|
Warning Review limit reached
Next review available in: 41 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThe README now presents the project as the Credo Controller REST API. It adds local, Docker, PostgreSQL, environment-based, and direct-binary setup instructions. It also updates configuration examples, repository references, package imports, and terminology. ChangesCredo Controller documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
README.md (7)
47-49: Prefer platform-agnostic wording or add Windows/macOS notesStating that Ubuntu 20.04 + is the “preferred OS” might discourage (or confuse) Windows/macOS developers although Docker makes the instructions portable. Either soften the wording (“tested on …”) or add a short note that the Docker flow works cross-platform.
50-55: Pin to a tag or commit to ensure reproducible clones
maincan move unexpectedly. Consider cloning a specific tag/commit in the docs to guarantee readers get the version that matches the README.-git clone https://github.com/credebl/credo-controller.git -cd credo-controller +git clone --branch vX.Y.Z https://github.com/credebl/credo-controller.git +cd credo-controller
60-63: Clarify sample path & ensure path works when README is rendered on npmjsThe sample file lives in
samples/cliConfig.json, not/samples/…(the leading slash suggests an absolute path).
Also note that npm renders README at package root, where an absolute path won’t link correctly.
66-89: Mask secrets or use placeholders in public docsReal-looking secrets (
"my-secret-key","admin-password") invite copy-paste into production. Replace with obvious placeholders (e.g."<MY_SECRET_KEY>") or add a bold warning above the block.
64-64: Remove trailing colon in heading to satisfy Markdown-lintHeading
#### Example \my-config.json`:` triggers MD026.
Drop the trailing colon:-#### Example `my-config.json`: +#### Example `my-config.json`
91-94: Fix blank line inside blockquote (MD028)There’s an empty line between two quote blocks which markdown-lint flags. Collapse the quotes into one or remove the blank line.
-> Ensure `my-config.json` is placed at the root of the project directory. - -> Do not commit `my-config.json` to version control. It may contain sensitive credentials. +> Ensure `my-config.json` is placed at the root of the project directory. +> **Important:** Do not commit `my-config.json` to version control. It may contain sensitive credentials.
100-108: Unify list marker style & follow MD004Earlier lists use
-while this block uses*. Pick one style project-wide (most of the doc uses-).-* Map container port `3000` to your local machine. -* Mount the `my-config.json` configuration file into the container. -* Start the application with the specified configuration. +- Map container port `3000` to your local machine. +- Mount the `my-config.json` configuration file into the container. +- Start the application with the specified configuration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
64-64: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
92-92: Blank line inside blockquote
(MD028, no-blanks-blockquote)
106-106: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
107-107: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
108-108: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
|
@Aiyaret-Sandhu Do you mind having a look at the failing DCOs |
GHkrishna
left a comment
GHkrishna
left a comment
There was a problem hiding this comment.
We can also, maybe look at better formatting the readme, especially in having appropriate heading sizes.
Also, the image for AFJ can most probably be update to be taken from the newest credo-ts image, along with changing all the reference for AFJ to credo or credo-controller as per appropriate
|
@GHkrishna I have looked at the DCOs and will correct them, please review my replies to your queries and confirm if i should proceed with the changes. |
Yes sure, I've addressed those comments |
Signed-off-by: Aiyaret Sandhu <arshsandhuprofessional@gmail.com>
Signed-off-by: Aiyaret Sandhu <arshsandhuprofessional@gmail.com>
2c775d1 to
73758fa
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
README.md (1)
14-18: Fix broken licence badge linkThe badge still points to the old Aries-Framework repo. This gives the wrong licence and 404s if that repo moves.
- <a - href="https://raw.githubusercontent.com/hyperledger/aries-framework-javascript-ext/main/LICENSE" + <a + href="https://raw.githubusercontent.com/credebl/credo-controller/main/LICENSE"
♻️ Duplicate comments (1)
README.md (1)
157-161: Duplicate host-network limitation appears againSame comment as above: the pre-built-image example also relies on
--network host. Please align the guidance here with whatever fix you choose for Method 2.
🧹 Nitpick comments (2)
README.md (2)
64-71: Relax the hard-pinned Node.js versionLocking the docs to the patch release 18.19.0 is unnecessarily strict. It will quickly become stale, and users on 18.[20-x] (or the current 20 LTS) will assume the project is incompatible.
Recommend wording along the lines of “Node.js 18 LTS (tested on 18.19)” and add a note that 20 LTS is expected to work but is not part of CI yet.
40-43: Address markdown-lint warnings
http://localhost:3000/docsis flagged as a bare URL (MD034). Wrap it in angle brackets or an inline link to silence automated docs linters.See the documentation at <http://localhost:3000/docs>.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
README.md (2)
Learnt from: GHkrishna
PR: credebl/credo-controller#281
File: samples/cliConfig.json:46-46
Timestamp: 2025-07-08T07:51:37.348Z
Learning: Sample configuration files in samples/ directories appropriately contain placeholder values like "supersecret" for API keys and other secrets to demonstrate the expected configuration format.
Learnt from: GHkrishna
PR: credebl/credo-controller#281
File: src/authentication.ts:0-0
Timestamp: 2025-07-09T11:14:49.387Z
Learning: In the credo-controller project, the team prefers JWT token cache to not expire (no TTL) for their authentication implementation in src/authentication.ts.
🪛 markdownlint-cli2 (0.17.2)
README.md
40-40: Bare URL used
(MD034, no-bare-urls)
69-69: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🔇 Additional comments (1)
README.md (1)
107-112:--network hostis Linux-only
docker run --network host …fails on Docker Desktop for macOS and Windows. For cross-platform instructions either:
- switch to explicit port mapping (
-p 3000:3000 -p 8080:8080 …), or- keep the current flag but add a note that it works only on Linux/WSL.
Without this, new users on the two most common desktop platforms will hit a cryptic error.
|
I think this PR looks good to me. However, I'll need to check for the last statements regarding |
|
@Aiyaret-Sandhu I think commits do not have verified signatures. Can you please fix them? |
I'm not really sure why the verified mark won't show up here, but as per the DCO, the commits are signed. So we won't be needing to worry about signing them again |
- Use CREDEBL logo and correct repo references (agent-controller) - Recommend Node.js 20 LTS aligned with CI and Docker - Clarify optional PostgreSQL wallet pool tunables and valid JSON - Note --network host is Linux-only with port-mapping alternative - Restore configuration docs (--help, JSON config, AFJ_REST env vars) - Fix /docs port to match sample config and note legacy binary name Signed-off-by: Ajay Jadhav <ajay@ayanworks.com>
- Update product name references and local image/tag names - Note that the published Docker image still uses the legacy credo-controller name Signed-off-by: Ajay Jadhav <ajay@ayanworks.com>
|
3 participants
This pull request updates the README.md file to improve clarity and correctness of the setup instructions.
Changes Made
Summary by CodeRabbit
.envusage, and updated CLI examples.