Skip to content

fix: README first-run instructions reference a nonexistent keygen service - #3

Open
batuhankocyigit wants to merge 2 commits into
canopy-network:mainfrom
batuhankocyigit:fix/readme-stale-keygen-references
Open

fix: README first-run instructions reference a nonexistent keygen service#3
batuhankocyigit wants to merge 2 commits into
canopy-network:mainfrom
batuhankocyigit:fix/readme-stale-keygen-references

Conversation

@batuhankocyigit

Copy link
Copy Markdown

fix: README's first-run key-generation instructions reference commands that don't exist

The bug

The README's very first documented step for a new node ("## 1. Node
setup") tells the user to run:

docker compose run --rm --no-deps -it keygen

docker-compose.yml has no service named keygen -- services: only
defines node. Running this exact command against the repo as-is fails
with no such service: keygen.

Two more references to the same no-longer-existing setup drifted along
with it:

  • README: "the equivalent shortcuts are make init-keys and make up"
    -- there is no init-keys target in the Makefile; the actual target
    is gen-key (confirmed elsewhere in the same README, in the Operations
    table: make gen-key # first boot only: generate the validator key).
  • README "Repo layout" section: docker-compose.yml # node (+ keygen, run once via \docker compose run keygen`)`.
  • Makefile's own .PHONY line still lists init-keys, which doesn't
    correspond to any real target (the actual gen-key target isn't
    listed there).

This all points at the same history: the key-generation step used to be
a keygen compose service / init-keys make target, and got replaced
with a raw docker run ... new-validator-key invocation (which
docker-compose.yml already documents in a comment, and which
make gen-key already wraps) -- but the README's primary instructions
and the Makefile's .PHONY line were never updated to match.

The fix

  • Replace the broken docker compose run --rm --no-deps -it keygen with
    the actual working command (matching docker-compose.yml's own comment
    and what make gen-key runs).
  • Fix make init-keysmake gen-key in the shortcuts line.
  • Fix the stale keygen mention in "Repo layout".
  • Fix the now-inaccurate Skip `keygen` wording in the migration
    section to refer to the step generically instead of a command name that
    no longer exists.
  • Fix the Makefile's .PHONY line: init-keysgen-key.

Testing

  • make -n gen-key -- dry-run output matches the command now documented
    in the README exactly.
  • make help -- lists gen-key correctly (previously init-keys didn't
    show up here at all, since help parses ##-commented target lines,
    not .PHONY, so this specific symptom was invisible until someone
    actually tried make init-keys or docker compose run keygen).
  • grep -rn "keygen\|init-keys" --include="*.md" --include="Makefile" --include="*.yml" . -- zero remaining matches after the fix (previously
    matched in 4 places across README.md and Makefile).

Scope

Two files, docs + one Makefile line. No changes to docker-compose.yml,
any service definitions, or actual node behavior -- this only fixes the
instructions and target name to match what already works.

…vice

The documented first step, `docker compose run --rm --no-deps -it
keygen`, fails with "no such service: keygen" -- docker-compose.yml
only defines a `node` service. Same drift shows up in three more
places: `make init-keys` (no such target; the real one is `gen-key`),
the "Repo layout" comment, and the Makefile's own .PHONY line still
listing `init-keys` instead of `gen-key`.

Replace the broken command with the one docker-compose.yml already
documents in a comment and that `make gen-key` already wraps, and fix
every other stale keygen/init-keys reference to match.

Verified with `make -n gen-key` (matches the README command exactly)
and a full-repo grep for keygen/init-keys (zero remaining matches).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant