Skip to content

change default#745

Merged
damongolding merged 3 commits into
task/releasefrom
task/change-default-wind
May 7, 2026
Merged

change default#745
damongolding merged 3 commits into
task/releasefrom
task/change-default-wind

Conversation

@damongolding

@damongolding damongolding commented May 6, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • Wind speed now displays in configurable units (metres per second or miles per hour) based on your selected unit system.
    • Wind direction can optionally display as degrees in addition to compass direction.
  • Documentation

    • Updated configuration schema text to clarify wind shows both speed and direction; wind_direction now described as degrees.

@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

Failed to post review comments

Walkthrough

Updates wind rendering and related docs: template adds wind formatting and unit selection with optional degree display; weather package adds an exported sentinel constant; tests updated to use the constant; schema descriptions for wind speed/direction clarified. No schema types or required fields changed.

Changes

Wind display + weather sentinel

Layer / File(s) Summary
Schema Documentation
config.schema.json
Textual edits: wind description now references both wind speed and wind direction; wind_direction description now documents degrees.
Core Symbol
internal/weather/weather.go
Introduces exported constant VarCompassDirection = "Var" and returns it from CompassDirection() when degrees are out of range.
Template Logic
internal/templates/partials/weather.templ
Adds formatWind helper, introduces windUnit selection (m/s vs mph), and restructures wind rendering to optionally show degrees and use the new formatter/unit.
Tests
internal/weather/weather_test.go
Updates expectations to use VarCompassDirection and adds test cases for negative and overflow degrees.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

enhancement

Poem

"I hopped through code on breezy nights,

I numbered gusts and tuned their flights,
In metres or in miles I play,
Degrees now gleam to show the way,
A rabbit cheers the wind's bright sway." 🐰💨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'change default' is vague and does not clearly describe the actual changes, which involve wind formatting, units, and schema updates for weather display. Consider revising the title to be more specific, such as 'Update wind display formatting and units' or 'Refactor wind speed formatting with unit switching', to better reflect the actual changes made.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task/change-default-wind

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
internal/templates/partials/weather.templ (2)

139-148: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Format weatherData.Wind.Speed consistently with temperature values.

The Wind.Speed field is a float64 that is rendered directly without formatting, producing unformatted precision (e.g., "3.6000000000001 m/s") instead of a clean value. The existing formatTemp function already handles this pattern for temperatures—applying similar formatting here would improve consistency.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/templates/partials/weather.templ` around lines 139 - 148, The
Wind.Speed float is rendered raw; change the template to format it like
temperatures by passing weatherData.Wind.Speed through the existing formatTemp
helper (e.g., replace the raw "{ weatherData.Wind.Speed }" with a call to
formatTemp(weatherData.Wind.Speed)) so the output shows a clean, rounded value
followed by the windUnit; keep the surrounding bits
(weatherData.Wind.CompassDirection(), windUnit, and the Show.WindDirection
conditional) unchanged.

140-143: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Normalise Unit field to lowercase before comparison.

The case-sensitive check weatherData.Unit == "imperial" at line 141 will silently fall back to "m/s" if the user provides "Imperial", "IMPERIAL", or any other case variation in their configuration. There is no normalisation of the Unit field throughout the data flow (config → WeatherLocation → WeatherData), and the constants MetricSystem and ImperialSystem defined in weather.go are not used in the comparison logic. If case-insensitive unit values are supported, apply strings.ToLower() to the Unit field during assignment or before the comparison.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/templates/partials/weather.templ` around lines 140 - 143, Normalize
the Unit value to lowercase before comparing so case variants like "Imperial"
match; update the assignment/usage of weatherData.Unit (or normalize in
WeatherLocation/WeatherData construction) using strings.ToLower() and then
compare against the defined constants MetricSystem/ImperialSystem (or their
lowercase equivalents) instead of the current case-sensitive check in the
template where windUnit is set; ensure comparisons reference the symbols
weatherData.Unit, ImperialSystem, and MetricSystem.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@internal/templates/partials/weather.templ`:
- Around line 139-148: The Wind.Speed float is rendered raw; change the template
to format it like temperatures by passing weatherData.Wind.Speed through the
existing formatTemp helper (e.g., replace the raw "{ weatherData.Wind.Speed }"
with a call to formatTemp(weatherData.Wind.Speed)) so the output shows a clean,
rounded value followed by the windUnit; keep the surrounding bits
(weatherData.Wind.CompassDirection(), windUnit, and the Show.WindDirection
conditional) unchanged.
- Around line 140-143: Normalize the Unit value to lowercase before comparing so
case variants like "Imperial" match; update the assignment/usage of
weatherData.Unit (or normalize in WeatherLocation/WeatherData construction)
using strings.ToLower() and then compare against the defined constants
MetricSystem/ImperialSystem (or their lowercase equivalents) instead of the
current case-sensitive check in the template where windUnit is set; ensure
comparisons reference the symbols weatherData.Unit, ImperialSystem, and
MetricSystem.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3e7bb336-1779-48f7-9afa-b954d17f6cfc

📥 Commits

Reviewing files that changed from the base of the PR and between b04c786 and b072803.

📒 Files selected for processing (2)
  • config.schema.json
  • internal/templates/partials/weather.templ

@damongolding damongolding merged commit 1024d3d into task/release May 7, 2026
3 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request May 8, 2026
@damongolding damongolding deleted the task/change-default-wind branch June 15, 2026 08:44
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