crueber/hello-team
Watch
1
0
Fork
You've already forked hello-team
 0
Code Issues 1 Pull requests Projects Releases 1 Packages Wiki Activity Actions

feat: hello CLI #2

Merged
crueber merged 2 commits from feat/hello-cli into main 2026-09-06 17:47:18 +00:00
Conversation 4 Commits 2 Files changed 8 +108 -1
crueber commented 2026-09-06 17:39:47 +00:00
Owner
Copy link

Implements issue #1: hello Go CLI. Prints hello from <name> (first CLI arg, default team). Includes Greet/pickName unit tests, Makefile (build/test), README, and go.mod.

Implements issue #1: hello Go CLI. Prints `hello from <name>` (first CLI arg, default `team`). Includes Greet/pickName unit tests, Makefile (build/test), README, and go.mod.
crueber commented 2026-09-06 17:42:04 +00:00
Author
Owner
Copy link

Code Review + Security Audit — PR #2 (feat/hello-cli)

Verdict: APPROVE ✅ — no majors. Verified locally: go build ./..., go vet, gofmt -l clean; go test ./... passes; make build produces bin/hello; runtime behavior matches spec (hello from team / hello from alice).

Correctness vs issue #1

  • Greet(name) returns hello from <name>, empty → team ✓
  • pickName(os.Args) uses first positional arg, defaults to team, extra args ignored ✓
  • Greet logic in testable pure function, not inline in main ✓
  • README documents usage + build; Makefile has build/test + .PHONY; bin/ gitignored ✓

Tests

Table-driven TestGreet covers default/empty, "alice", "world". TestPickName covers no-arg, one-arg, extra-args-ignored. Good coverage for scope.

Security audit

  • No command injection surface: no exec, no shell, no flags. Output is a single fmt.Println of a composed string — no format-string injection (value passed as Println arg, not as format).
  • No path handling beyond go build -o bin/hello (build-time only).
  • No secrets, no network, no unsafe flags, no cgo. Low-risk tool; nothing to fix.

Minors (non-blocking)

  1. Greet("") and pickName both default to "team" — slight duplication of the default; acceptable, but a shared constant (defaultName) would keep them in sync.
  2. Test cases could use subtests (t.Run) for clearer failure names; cosmetic.
  3. Consider make test also running go vet ./... in CI pipelines.

Nice, clean implementation — merging is safe.

## Code Review + Security Audit — PR #2 (feat/hello-cli) **Verdict: APPROVE** ✅ — no majors. Verified locally: `go build ./...`, `go vet`, `gofmt -l` clean; `go test ./...` passes; `make build` produces `bin/hello`; runtime behavior matches spec (`hello from team` / `hello from alice`). ### Correctness vs issue #1 - `Greet(name)` returns `hello from <name>`, empty → `team` ✓ - `pickName(os.Args)` uses first positional arg, defaults to `team`, extra args ignored ✓ - Greet logic in testable pure function, not inline in `main` ✓ - README documents usage + build; Makefile has `build`/`test` + `.PHONY`; `bin/` gitignored ✓ ### Tests Table-driven `TestGreet` covers default/empty, "alice", "world". `TestPickName` covers no-arg, one-arg, extra-args-ignored. Good coverage for scope. ### Security audit - No command injection surface: no `exec`, no shell, no flags. Output is a single `fmt.Println` of a composed string — no format-string injection (value passed as Println arg, not as format). - No path handling beyond `go build -o bin/hello` (build-time only). - No secrets, no network, no unsafe flags, no cgo. Low-risk tool; nothing to fix. ### Minors (non-blocking) 1. `Greet("")` and `pickName` both default to "team" — slight duplication of the default; acceptable, but a shared constant (`defaultName`) would keep them in sync. 2. Test cases could use subtests (`t.Run`) for clearer failure names; cosmetic. 3. Consider `make test` also running `go vet ./...` in CI pipelines. Nice, clean implementation — merging is safe.
crueber commented 2026-09-06 17:44:57 +00:00
Author
Owner
Copy link

Builder addressed all 3 review minors (commit 498f59e): shared DefaultName constant, t.Run subtests in both test files, doc comment polish. go test ./..., make test, make build all pass; go vet and gofmt -l clean. Binary verified: hello from team / hello from alice.

Builder addressed all 3 review minors (commit 498f59e): shared DefaultName constant, t.Run subtests in both test files, doc comment polish. go test ./..., make test, make build all pass; go vet and gofmt -l clean. Binary verified: hello from team / hello from alice.
crueber commented 2026-09-06 17:46:12 +00:00
Author
Owner
Copy link

QA Report — PR #2 (feat/hello-cli) vs Issue #1

Verdict: PASS ✅ (head commit 498f59e)

# Check Result Observed output
1 make build produces working binary PASS go build -o bin/hello . — binary runs
2 No args → hello from team PASS hello from team (exact match)
3 Arg alice → hello from alice PASS hello from alice (exact match)
4 Extra args ignored PASS ./bin/hello alice bob carol → hello from alice
5 make test passes PASS ok hello-team 0.001s (6 subtests: TestGreet ×3, TestPickName ×3)
6 go test ./... passes PASS ok hello-team 0.001s
7 README.md documents build + usage PASS Sections: Build (make build), Test (make test), Usage (./bin/hello, ./bin/hello alice)
8 Makefile has build + test targets PASS build: go build -o bin/hello . and test: go test ./... -v

All issue #1 acceptance criteria met.

## QA Report — PR #2 (feat/hello-cli) vs Issue #1 **Verdict: PASS** ✅ (head commit `498f59e`) | # | Check | Result | Observed output | |---|-------|--------|-----------------| | 1 | `make build` produces working binary | PASS | `go build -o bin/hello .` — binary runs | | 2 | No args → `hello from team` | PASS | `hello from team` (exact match) | | 3 | Arg `alice` → `hello from alice` | PASS | `hello from alice` (exact match) | | 4 | Extra args ignored | PASS | `./bin/hello alice bob carol` → `hello from alice` | | 5 | `make test` passes | PASS | `ok hello-team 0.001s` (6 subtests: TestGreet ×3, TestPickName ×3) | | 6 | `go test ./...` passes | PASS | `ok hello-team 0.001s` | | 7 | README.md documents build + usage | PASS | Sections: Build (`make build`), Test (`make test`), Usage (`./bin/hello`, `./bin/hello alice`) | | 8 | Makefile has build + test targets | PASS | `build: go build -o bin/hello .` and `test: go test ./... -v` | All issue #1 acceptance criteria met.
crueber commented 2026-09-06 17:47:19 +00:00
Author
Owner
Copy link

Merged by chiefdev after reviewer APPROVE + qa PASS. Shipping release v1.0.0.

Merged by chiefdev after reviewer APPROVE + qa PASS. Shipping release v1.0.0.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
changes-requested

   
in-review

   
plan-approved

   
plan-rejected

   
qa-fail

   
qa-pass
No labels
changes-requested
in-review
plan-approved
plan-rejected
qa-fail
qa-pass
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
crueber/hello-team!2
No description provided.