feat: add wait grp exercise #126
Conversation
WalkthroughAdds a new WaitGroups exercise: catalog entry with hints, a template implementation and test under templates/39_waitgroup, and a reference solution under solutions/39_waitgroup. Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test (waitgroup_test.go)
participant F as waitGroup()
participant WG as sync.WaitGroup
participant W as worker()
T->>F: call waitGroup()
activate F
F->>WG: Add(1)
F->>W: start goroutine(worker, &WG, &result)
note over W: set result = "Worker done"\nWG.Done()
F->>WG: Wait()
F-->>T: return result
deactivate F
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 1
🧹 Nitpick comments (5)
internal/exercises/catalog.yaml (3)
208-209: Refine topics for discoverability/consistency.Recommend: ["concurrency", "sync", "goroutines", "waitgroup"].
- topics: ["waitGroup", "go"] + topics: ["concurrency", "sync", "goroutines", "waitgroup"]
210-210: Fix grammar and provide more actionable hints.Suggested hints:
- "Use sync.WaitGroup to wait for goroutines."
- "Call wg.Add(n) before starting n goroutines; each must call wg.Done()."
- "Call wg.Wait() to block until completion."
- hints: - - "Use Go's waitGroups inorder to wait for the goroutines" + hints: + - "Use sync.WaitGroup to wait for goroutines." + - "Call wg.Add(n) before starting n goroutines; each must call wg.Done()." + - "Call wg.Wait() in the parent to block until completion."
205-205: Use lowercase slug for consistency
Changeslug: 37_waitGrouptoslug: 37_waitgroup.internal/exercises/solutions/37_waitGroup/waitGroup.go (1)
7-7: Remove stale TODO in solution file.The functions are implemented; drop the TODO to avoid confusion.
-// TODO: Implement these functions so tests passinternal/exercises/templates/37_waitGroup/waitGroup_test.go (1)
5-5: Rename test for clarity.Use a descriptive name.
-func TestHello(t *testing.T) { +func TestWaitGroup(t *testing.T) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/exercises/catalog.yaml(1 hunks)internal/exercises/solutions/37_waitGroup/waitGroup.go(1 hunks)internal/exercises/templates/37_waitGroup/waitGroup.go(1 hunks)internal/exercises/templates/37_waitGroup/waitGroup_test.go(1 hunks)
🔇 Additional comments (4)
internal/exercises/solutions/37_waitGroup/waitGroup.go (2)
11-14: Correct use of WaitGroup and result handoff.Goroutine signals Done and writes result; parent waits before return. LGTM.
16-22: Main flow is idiomatic and safe.Add(1) → start goroutine → Wait() → return result. Good.
internal/exercises/templates/37_waitGroup/waitGroup_test.go (1)
6-9: CI does not run tests; template tests aren’t executed
The workflows only perform formatting, vetting, and building—nogo teststeps run on PRs, so the template tests won’t break CI.Likely an incorrect or invalid review comment.
internal/exercises/templates/37_waitGroup/waitGroup.go (1)
9-17: Do not synchronize in the template
The code in internal/exercises/templates/37_waitGroup is intentionally incomplete; tests are meant to fail until students add wg.Add(1), wg.Done(), and wg.Wait().Likely an incorrect or invalid review comment.
internal/exercises/catalog.yaml
Outdated
| - slug: 37_waitGroup | ||
| title: "WaitGroups" | ||
| difficulty: beginner | ||
| topics: ["waitGroup", "go"] | ||
| hints: |
There was a problem hiding this comment.
Place “WaitGroups” under concepts, not projects (issue #67 says Concept).
Move this entry to the concepts section to match the issue’s acceptance criteria.
🤖 Prompt for AI Agents
In internal/exercises/catalog.yaml around lines 205 to 209, the "WaitGroups"
exercise is currently listed under projects but should be placed under the
concepts section per issue #67; move the entire exercise entry (slug, title,
difficulty, topics, hints) from the projects list into the concepts list,
preserving its fields and indentation to match surrounding YAML entries, and
remove the original entry from the projects section so there are no duplicates.
|
@kush1jpeg: PR LGTM, can you fix the conflicts? |
|
@zhravan done. |
|
@kush1jpeg Thank you for the effort, this Example: File Downloader Solution func DownloadFiles(urls []string) []string {
var wg sync.WaitGroup
results := make([]string, len(urls))
for i, url := range urls {
wg.Add(1)
go func(i int, url string) {
defer wg.Done()
time.Sleep(10 * time.Microsecond)
results[i] = fmt.Sprintf("Downloaded %s", url)
}(i, url)
}
wg.Wait()
return results
}I'm okay if you used this exact example. Also in exercise code have meaningful TODO comments like here Also make all the exercise, solution, catalog changes a single commit. Or you can squash after reviewing. @zhravan What do you think? |
|
@kaushalyap : Commits shouldnt matter, we can squash and merge to main. This should be okay. Yes let's keep the TODO comments pattern for all exercises, easier for one who use it. |
| "sync" | ||
| ) | ||
|
|
||
| // TODO: Implement these functions so tests pass |
There was a problem hiding this comment.
Give one liner hints if possible.
3 participants
Summary
Added templates under internal/exercises/templates/:slug
Added tests under internal/exercises/templates/:slug
Added to internal/exercises/catalog.yaml and provided hints
Fixes #67
Summary by CodeRabbit