Skip to content

Fixed incorrect package name in panic exercise which caused go vet error in CI - #169

Merged
kawpii merged 3 commits into
zhravan:mainfrom
kawpii:fix-go-vet-build-failure
Nov 17, 2025
Merged

Fixed incorrect package name in panic exercise which caused go vet error in CI#169
kawpii merged 3 commits into
zhravan:mainfrom
kawpii:fix-go-vet-build-failure

Conversation

@kawpii

@kawpii kawpii commented Nov 17, 2025

Copy link
Copy Markdown
Collaborator

Summary

As described in the title.

Checklist

  • Tests pass: make verify or golearn verify <slug>
  • Docs updated (README/CONTRIBUTING) if needed
  • No large new dependencies

Summary by CodeRabbit

  • Refactor

    • Internal package and public API naming updated for cleaner organization.
    • Removed an unused concurrency dependency to simplify internals.
  • Bug Fixes

    • Relaxed a timing assertion in tests to reduce flakiness.

Note: No user-facing features added; changes are internal and aim to improve code structure and test stability.

@coderabbitai

coderabbitai Bot commented Nov 17, 2025

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

Walkthrough

Renames and small refactors across exercise templates: one test package renamed, several time-delay template functions renamed to their final exported names (implementations remain TODO), removal of an unused sync import/WaitGroup, and a relaxed timing assertion in a worker-pools test.

Changes

Cohort / File(s) Change Summary
Package rename
internal/exercises/templates/39_panic/panic_test.go
Package declaration changed from package exercises to package panicex.
Time-delay API renames
internal/exercises/templates/41_time_delay/time_delay.go
Multiple template function names and signatures renamed to final exported forms (e.g., WaitForTemplateWaitFor, NotifyAfterTemplateNotifyAfter, ScheduleAfterTemplateScheduleAfter, ElapsedMillisTemplateElapsedMillis, etc.). Implementations remain TODO.
Wait-group cleanup
internal/exercises/templates/42_wait_group/wait_group.go
Removed sync import and an unused WaitGroup variable from Squares; function still returns nil.
Test timing relaxation
internal/exercises/templates/43_worker_pools/worker_pools_test.go
Increased allowed upper bound for a timing assertion in TestLogProcessor (now up to ~1400μs) with comment explaining flakiness/observed max.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant TimeDelay as timedelay (package)
    rect rgb(230, 248, 230)
    Note right of TimeDelay: API surface (renamed functions)
    end
    Caller->>TimeDelay: WaitFor(ms)
    Caller->>TimeDelay: NotifyAfter(ms) --> returns chan bool
    Caller->>TimeDelay: NotifyAt(target) --> returns chan bool
    Caller->>TimeDelay: ScheduleAfter(ms, fn) --> returns chan struct{}
    Note over Caller,TimeDelay: Implementations are TODO (signatures changed only)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Multiple related but distinct edits across 4 files; mostly straightforward renames and a minor test tweak.
  • Areas to pay attention to:
    • Ensure package rename in panic_test.go matches any other test or package references.
    • Confirm time-delay function renames are consistent with callers elsewhere in the repo (public API change).
    • Verify removal of sync and wg does not affect other logic or expected concurrency behavior.
    • Check test timing change is intentional and doesn’t mask real regressions.

Poem

🐇 I hopped through code at break of day,

Renamed a package, tuned delays to play,
I trimmed a WaitGroup and loosened a test,
Little changes — but I feel blessed! 🎈

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing an incorrect package name in the panic exercise that caused a go vet error in CI, which matches the file-level change from 'package exercises' to 'package panicex'.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ca11750 and c515131.

📒 Files selected for processing (3)
  • internal/exercises/templates/41_time_delay/time_delay.go (1 hunks)
  • internal/exercises/templates/42_wait_group/wait_group.go (0 hunks)
  • internal/exercises/templates/43_worker_pools/worker_pools_test.go (1 hunks)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kawpii kawpii added the patch Bug fixes and small improvements label Nov 17, 2025
@kawpii
kawpii merged commit ac12867 into zhravan:main Nov 17, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch Bug fixes and small improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant