Add feature: integrate weatherxu API#204
Conversation
tinhuynh1
force-pushed
the
feature/integrate-weatherxu-api
branch
from
6baddd8 to
d25a66e
Compare
tinhuynh1
force-pushed
the
feature/integrate-weatherxu-api
branch
from
d25a66e to
1b9a20b
Compare
|
Please rebase onto latest master. |
There was a problem hiding this comment.
Pull request overview
Adds a new WeatherXu backend to fetch and render forecasts in wego, and documents how to configure it via .wegorc.
Changes:
- Added a
weatherxubackend implementation (API client + response parsing + backend registration). - Updated README setup instructions to include WeatherXu account/config guidance.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| README.md | Documents how to configure wego to use the new weatherxu backend. |
| backends/weatherxu.go | Implements the WeatherXu API integration and registers the backend under iface.AllBackends["weatherxu"]. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| debug bool | ||
| } | ||
|
|
||
| // WeatherXuResponse represents the main response structure |
There was a problem hiding this comment.
The comment says "WeatherXuResponse" but the actual type is weatherXuResponse (unexported). This is confusing and will trigger golint-style checks; either rename the type to WeatherXuResponse or update/remove the comment so it matches the identifier.
| // WeatherXuResponse represents the main response structure | |
| // weatherXuResponse represents the main response structure |
| func (c *weatherXuConfig) Setup() { | ||
| flag.StringVar(&c.apiKey, "weatherxu-api-key", "", "weatherxu backend: the api `KEY` to use") | ||
| flag.StringVar(&c.lang, "weatherxu-lang", "en", "weatherxu backend: the `LANGUAGE` to request from weatherxu") | ||
| flag.BoolVar(&c.debug, "weatherxu-debug", false, "weatherxu backend: print raw requests and responses") |
There was a problem hiding this comment.
-weatherxu-lang is defined and stored on the config, but c.lang is never used when building the WeatherXu request. Either remove the flag/config field, or pass the language through to the API (query parameter/header) so the option actually changes the response.
| s := strings.Split(location, ",") | ||
| loc = fmt.Sprintf("lat=%s&lon=%s", s[0], s[1]) | ||
| requestUrl := fmt.Sprintf(weatherXuURI, loc) | ||
| resp, err := c.fetch(requestUrl) | ||
| if err != nil { |
There was a problem hiding this comment.
numdays is never applied: the request URL does not include any forecast-days/limit parameter, and the parsed forecast is returned as-is. This breaks the global -days/config behavior; either request only numdays from WeatherXu (if supported) or trim ret.Forecast to numdays before returning.
| if matched, err := regexp.MatchString(`^-?[0-9]*(\.[0-9]+)?,-?[0-9]*(\.[0-9]+)?$`, location); !matched || err != nil { | ||
| log.Fatalf("Error: The weatherxu backend only supports latitude,longitude pairs as location %s.\n", location) | ||
| } |
There was a problem hiding this comment.
The latitude/longitude validation regex allows empty values (e.g. "," matches because [0-9]* can be empty). Consider requiring at least one digit for both lat and lon (use + instead of *) so invalid locations fail fast with a clear error.
Motivation and Context
Description
Steps for Testing
Screenshots