oss: clean up internal developer references for OSS readiness - #20
Conversation
|
Reviewed for OSS-readiness. Completeness and leak scans are clean (no Applied two fixes (pushed to this branch):
Also updated the PR description with a Breaking changes note: the keyring service rename ( |
ba24afa to
74d2d73
Compare
There was a problem hiding this comment.
Pull request overview
Prepares rocm-cli for open-source release by removing internal developer identifiers (usernames and home directory paths) and updating public-facing links/identifiers to ROCm-owned, neutral values.
Changes:
- Updated repository/download references (Cargo metadata, README, install scripts) to
https://github.com/ROCm/rocm-cli. - Replaced developer-specific filesystem paths in scripts, docs, and test fixtures with generic examples.
- Renamed keyring service ID and refreshed a few runtime/config path-related identifiers for OSS readiness.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/single_exe_release_gate.py | Removes hardcoded WSL user/path from smoke command invocation. |
| scripts/rust_cosmopolitan_spike.py | Updates Windows guidance message to avoid referencing a specific WSL user. |
| scripts/build_single_exe_release.py | Replaces developer-specific Windows llvm-strip.exe probe paths with generic placeholders. |
| README.md | Updates bootstrap installer URLs to ROCm/rocm-cli. |
| plans/rocm-cli-remaining-implementation-plan.md | Replaces developer home paths in example commands. |
| plans/cosmopolitan-runtime-boundary-plan.md | Replaces developer-specific Windows/Linux example paths. |
| install.sh | Defaults GitHub repo to ROCm/rocm-cli. |
| install.ps1 | Defaults GitHub repo to ROCm/rocm-cli. |
| engines/vllm/src/lib.rs | Replaces developer home paths in test fixtures. |
| engines/sglang/src/lib.rs | Replaces developer home paths in test fixtures. |
| engines/lemonade/src/lib.rs | Replaces developer-specific Windows paths in tests. |
| engines/atom/src/lib.rs | Replaces developer home paths in test fixtures. |
| docs/wsl.md | Replaces developer-specific venv path example. |
| docs/testing.md | Replaces developer home paths in testing commands. |
| docs/sglang.md | Replaces developer home path in acceptance harness example. |
| docs/implementation-completion-audit.md | Replaces developer paths in WSL verification snippets. |
| docs/current-tui-therock-correction-plan.md | Removes internal branch reference; replaces WSL command paths. |
| docs/cosmopolitan-universal-binary-plan.md | Removes internal branch reference. |
| docs/atom.md | Replaces developer home path in acceptance harness example. |
| crates/rocm-core/src/runtime.rs | Updates ProjectDirs org/qualifier and refreshes path-related test fixtures. |
| Cargo.toml | Updates repository field to https://github.com/ROCm/rocm-cli. |
| apps/rocm/src/tui.rs | Replaces developer-specific Windows/WSL paths in help text and tests. |
| apps/rocm/src/therock.rs | Replaces developer-specific Windows/WSL paths in tests. |
| apps/rocm/src/provider_keys.rs | Renames keyring service ID to a neutral identifier. |
| apps/rocm/src/main.rs | Replaces developer-specific install path examples in assistant text and tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
juhovainio
left a comment
There was a problem hiding this comment.
@rominf lgtm, but pls double check the co-pilot comments before merge
3b8c82a to
7ae3f53
Compare
Replace personal developer identifiers that would be inappropriate in a public repository: - Cargo.toml, README.md, install.sh, install.ps1: update repository and download URLs from powderluv/rocm-cli to ROCm/rocm-cli - apps/rocm/src/provider_keys.rs: rename keyring service ID from powderluv.rocm-cli.provider-key to org.rocm.rocm-cli.provider-key (reverse-DNS convention; avoids collisions with other software) - crates/rocm-core/src/runtime.rs: update ProjectDirs qualifier/org from com/powderluv to org/ROCm (relocates on-disk config dirs; intentional pre-release break) - Engine test fixtures (atom, sglang, vllm, lemonade), app sources (main.rs, tui.rs, therock.rs), docs and plans: replace developer home paths (/home/jam/.venv/, D:\jam\...) with generic examples Breaking: the keyring service rename and ProjectDirs qualifier change orphan any provider API keys or config dirs from a pre-existing install. Affected users must re-run `rocm config set-provider-key <provider>`.
7ae3f53 to
c9a6b7a
Compare
Summary
Prepares the repository for public release by removing personal developer identifiers and placeholder paths.
Changes
apps/rocm/src/provider_keys.rs): replaced personal GitHub username with namespaced neutral identifierorg.rocm.rocm-cli.provider-keyCargo.toml): updatedrepositoryfield tohttps://github.com/ROCm/rocm-cliREADME.md,install.sh,install.ps1): updated download URLs to point toROCm/rocm-clicrates/rocm-core/src/runtime.rs): updatedProjectDirsqualifier/org toorg/ROCmengines/atom,engines/sglang,engines/vllm,engines/lemonade): replaced developer home paths (/home/jam/.venv/,D:/jam/...) with generic examplesapps/rocm/src/main.rs,tui.rs,therock.rs): replaced developer-specific Windows/WSL paths with generic examplesscripts/): replaced hardcoded developer paths with portable equivalents;single_exe_release_gate.pynow anchors the WSL--cddirectory toREPO_ROOT(via the existingwsl_path()helper) instead of a hardcoded pathBreaking changes
powderluv.rocm-cli.provider-key→org.rocm.rocm-cli.provider-key): the OS keyring stores provider API keys under this service name, so any key saved by a pre-existing install will no longer be found after upgrading. Affected users must re-save their key withrocm config set-provider-key <provider>(or set the provider's API-key environment variable). No data is lost; the old entry simply becomes orphaned. Acceptable for a pre-OSS-release rename, but noted here for anyone running an earlier build.Test Plan
cargo check --workspacepassescargo test --workspace— 750/751 tests pass (1 pre-existing flaky test in TUI, confirmed on main)cargo clippy --workspace -- -D warnings— zero warningscargo fmt --check --all— clean (formatting fix committed as part of this PR)powderluv,jamuser references remain in any tracked file