Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughBumps release/version metadata to v1.9.2 across CI, packaging, and docs; adds Gender Rate, Egg Cycles, and Effort Values to Pokémon CLI output (with corresponding struct fields and golden test updates); and replaces a short argument-error message with a styled multi-line error block. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Merging this PR will not alter performance
Comparing Footnotes
|
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
115-121:⚠️ Potential issue | 🟡 MinorUpdate stale Docker image tag references to v1.9.2.
The Docker commands in this NOTE section still reference
v1.9.1instead ofv1.9.2. These should be updated for consistency with the other Docker references updated in this PR (lines 5, 102, 106).🐛 Proposed fix
> # Kitty -> docker run --rm -it -e TERM -e KITTY_WINDOW_ID digitalghostdev/poke-cli:v1.9.1 card +> docker run --rm -it -e TERM -e KITTY_WINDOW_ID digitalghostdev/poke-cli:v1.9.2 card > > # WezTerm, iTerm2, Ghostty, Konsole, Rio, Tabby -> docker run --rm -it -e TERM -e TERM_PROGRAM digitalghostdev/poke-cli:v1.9.1 card +> docker run --rm -it -e TERM -e TERM_PROGRAM digitalghostdev/poke-cli:v1.9.2 card > > # Windows Terminal (Sixel) -> docker run --rm -it -e WT_SESSION digitalghostdev/poke-cli:v1.9.1 card +> docker run --rm -it -e WT_SESSION digitalghostdev/poke-cli:v1.9.2 card🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 115 - 121, Update the three docker run examples that currently use the image tag digitalghostdev/poke-cli:v1.9.1 to use v1.9.2 instead; specifically replace v1.9.1 with v1.9.2 in the lines beginning with "docker run --rm -it -e TERM -e KITTY_WINDOW_ID digitalghostdev/poke-cli:v1.9.1 card", "docker run --rm -it -e TERM -e TERM_PROGRAM digitalghostdev/poke-cli:v1.9.1 card", and "docker run --rm -it -e WT_SESSION digitalghostdev/poke-cli:v1.9.1 card" so they become consistent with the other updated Docker references.
🧹 Nitpick comments (2)
cmd/utils/validateargs.go (1)
45-45: Prefer neutral phrasing for the missing-name message.
"a(n) %s's name"reads awkwardly for some command names. A neutral template is cleaner and more reusable.Proposed wording tweak
- fmt.Sprintf("\nPlease declare a(n) %s's name after the <%s> command", v.CmdName, v.CmdName), + fmt.Sprintf("\nPlease provide the required name after the <%s> command", v.CmdName),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/utils/validateargs.go` at line 45, Replace the awkward "a(n) %s's name" phrasing with a neutral template: update the format string that builds the missing-name message (where fmt.Sprintf is called with v.CmdName twice) to something like "Please provide a name for %s when using the <%s> command" so it references v.CmdName and the command token cleanly; adjust the fmt.Sprintf call that constructs the message to use this new template.cmd/pokemon/pokemon.go (1)
99-113: Avoid rebuildingmodernEggInformationNamesinside the loop.
Define the map once before iterating to reduce redundant allocations.♻️ Proposed refactor
- for _, entry := range pokemonSpeciesStruct.EggGroups { - modernEggInformationNames := map[string]string{ - "indeterminate": "Amorphous", - "ground": "Field", - "humanshape": "Human-Like", - "plant": "Grass", - "no-eggs": "Undiscovered", - } + modernEggInformationNames := map[string]string{ + "indeterminate": "Amorphous", + "ground": "Field", + "humanshape": "Human-Like", + "plant": "Grass", + "no-eggs": "Undiscovered", + } + + for _, entry := range pokemonSpeciesStruct.EggGroups {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/pokemon/pokemon.go` around lines 99 - 113, The map modernEggInformationNames is being recreated on every iteration of the loop over pokemonSpeciesStruct.EggGroups; move its declaration out of the loop and define it once before the for _, entry := range pokemonSpeciesStruct.EggGroups loop (either as a local variable just above the loop or as a package-level var if appropriate), then inside the loop simply look up entry.Name and append to eggInformationSlice (falling back to cases.Title(language.English).String(entry.Name) when missing).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/pokemon/pokemon.go`:
- Around line 118-165: The gender and stat label lookups can produce blank
strings for unexpected keys; update the gender handling around
pokemonSpeciesStruct.GenderRate (map m) to use a safe lookup (val, ok :=
m[genderRate]) and set a clear fallback like "Unknown" or a formatted numeric
label when !ok; likewise in the effortValues closure use a safe lookup from
nameMapping for effortValue.Stat.Name (name, ok := nameMapping[...]) and when
missing fall back to a readable form (e.g., the original stat name or a
Title-cased version) before appending to evs so no empty labels are emitted.
In `@docs/nginx.conf`:
- Line 6: Remove the unnecessary GitHub API allowance from the
Content-Security-Policy add_header directive: edit the add_header
Content-Security-Policy line to drop "https://api.github.com" from the
connect-src list since the site does not call the GitHub API; if you later add a
feature that requires GitHub API access, re-add that origin and include an
inline comment on the same add_header line explaining why it is required.
---
Outside diff comments:
In `@README.md`:
- Around line 115-121: Update the three docker run examples that currently use
the image tag digitalghostdev/poke-cli:v1.9.1 to use v1.9.2 instead;
specifically replace v1.9.1 with v1.9.2 in the lines beginning with "docker run
--rm -it -e TERM -e KITTY_WINDOW_ID digitalghostdev/poke-cli:v1.9.1 card",
"docker run --rm -it -e TERM -e TERM_PROGRAM digitalghostdev/poke-cli:v1.9.1
card", and "docker run --rm -it -e WT_SESSION digitalghostdev/poke-cli:v1.9.1
card" so they become consistent with the other updated Docker references.
---
Nitpick comments:
In `@cmd/pokemon/pokemon.go`:
- Around line 99-113: The map modernEggInformationNames is being recreated on
every iteration of the loop over pokemonSpeciesStruct.EggGroups; move its
declaration out of the loop and define it once before the for _, entry := range
pokemonSpeciesStruct.EggGroups loop (either as a local variable just above the
loop or as a package-level var if appropriate), then inside the loop simply look
up entry.Name and append to eggInformationSlice (falling back to
cases.Title(language.English).String(entry.Name) when missing).
In `@cmd/utils/validateargs.go`:
- Line 45: Replace the awkward "a(n) %s's name" phrasing with a neutral
template: update the format string that builds the missing-name message (where
fmt.Sprintf is called with v.CmdName twice) to something like "Please provide a
name for %s when using the <%s> command" so it references v.CmdName and the
command token cleanly; adjust the fmt.Sprintf call that constructs the message
to use this new template.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e430c8f-c52b-4f76-ac28-3104173472ca
📒 Files selected for processing (22)
.github/workflows/ci.yml.github/workflows/python_testing.yml.goreleaser.ymlDockerfileREADME.mdcard_data/pipelines/poke_cli_dbt/dbt_project.ymlcmd/pokemon/pokemon.gocmd/utils/validateargs.godocs/nginx.confnfpm.yamlstructs/structs.gotestdata/main_latest_flag.goldentestdata/pokemon_abilities.goldentestdata/pokemon_defense.goldentestdata/pokemon_defense_ability_immunities.goldentestdata/pokemon_image.goldentestdata/pokemon_image_flag_non-valid_size.goldentestdata/pokemon_no_flags_dual_type.goldentestdata/pokemon_regional_form.goldentestdata/pokemon_regional_form_2.goldentestdata/pokemon_stats.goldenweb/pyproject.toml
💤 Files with no reviewable changes (1)
- .github/workflows/python_testing.yml
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cmd/pokemon/pokemon.go (1)
118-167:⚠️ Potential issue | 🟡 MinorAdd safe fallbacks for derived output fields.
Line 139 can render an empty gender value for unmapped
GenderRate, and Line 167 can render a blankEffort Values:when no EVs exist.🛡️ Proposed robustness fix
genderRate := pokemonSpeciesStruct.GenderRate m := map[int]string{ @@ } + genderRateText, ok := m[genderRate] + if !ok { + genderRateText = "Unknown" + } @@ fmt.Fprintf(w, "\n%s %s %s\n%s %s %s\n%s %s %d", @@ - "Gender Rate:", m[genderRate], + "Gender Rate:", genderRateText, @@ for _, effortValue := range pokemonStruct.Stats { @@ } - fmt.Fprintf(w, "\n%s Effort Values: %s", styling.ColoredBullet, strings.Join(evs, ", ")) + if len(evs) == 0 { + fmt.Fprintf(w, "\n%s Effort Values: None", styling.ColoredBullet) + return + } + fmt.Fprintf(w, "\n%s Effort Values: %s", styling.ColoredBullet, strings.Join(evs, ", ")) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/pokemon/pokemon.go` around lines 118 - 167, The output can show empty/missing values: when rendering gender rate use a safe lookup for genderRate against the map m (e.g., check if m[genderRate] exists and fall back to "Unknown" or "Genderless" instead of indexing blindly) and update the fmt.Fprintf call that prints Gender Rate to use that fallback; similarly in the effortValues function, if the evs slice is empty, render a clear fallback like "None" (or "0") instead of joining an empty slice so the "Effort Values:" line is never blank; reference symbols to change: genderRate, m, the fmt.Fprintf that prints Gender Rate, effortValues, evs, and the fmt.Fprintf that prints Effort Values.
🧹 Nitpick comments (1)
cmd/pokemon/pokemon.go (1)
99-107: Hoist static egg-name mapping out of the loop.
modernEggInformationNamesis recreated on every iteration. Defining it once before the loop is cleaner and avoids repeated allocations.♻️ Proposed refactor
- for _, entry := range pokemonSpeciesStruct.EggGroups { - modernEggInformationNames := map[string]string{ - "indeterminate": "Amorphous", - "ground": "Field", - "humanshape": "Human-Like", - "plant": "Grass", - "no-eggs": "Undiscovered", - } + modernEggInformationNames := map[string]string{ + "indeterminate": "Amorphous", + "ground": "Field", + "humanshape": "Human-Like", + "plant": "Grass", + "no-eggs": "Undiscovered", + } + for _, entry := range pokemonSpeciesStruct.EggGroups {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/pokemon/pokemon.go` around lines 99 - 107, The map modernEggInformationNames is being reallocated inside the loop over pokemonSpeciesStruct.EggGroups; move its declaration out of the loop so it's created once and reused. Specifically, define modernEggInformationNames (the mapping from "indeterminate","ground","humanshape","plant","no-eggs" to their modern names) before the for _, entry := range pokemonSpeciesStruct.EggGroups loop and then use that single map instance inside the loop when processing each entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cmd/pokemon/pokemon.go`:
- Around line 118-167: The output can show empty/missing values: when rendering
gender rate use a safe lookup for genderRate against the map m (e.g., check if
m[genderRate] exists and fall back to "Unknown" or "Genderless" instead of
indexing blindly) and update the fmt.Fprintf call that prints Gender Rate to use
that fallback; similarly in the effortValues function, if the evs slice is
empty, render a clear fallback like "None" (or "0") instead of joining an empty
slice so the "Effort Values:" line is never blank; reference symbols to change:
genderRate, m, the fmt.Fprintf that prints Gender Rate, effortValues, evs, and
the fmt.Fprintf that prints Effort Values.
---
Nitpick comments:
In `@cmd/pokemon/pokemon.go`:
- Around line 99-107: The map modernEggInformationNames is being reallocated
inside the loop over pokemonSpeciesStruct.EggGroups; move its declaration out of
the loop so it's created once and reused. Specifically, define
modernEggInformationNames (the mapping from
"indeterminate","ground","humanshape","plant","no-eggs" to their modern names)
before the for _, entry := range pokemonSpeciesStruct.EggGroups loop and then
use that single map instance inside the loop when processing each entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31b0c460-6cdc-4740-8a80-25c997ecd689
📒 Files selected for processing (2)
README.mdcmd/pokemon/pokemon.go
✅ Files skipped from review due to trivial changes (1)
- README.md
Summary by CodeRabbit
New Features
Chores
Security