Repository navigation
Update README - #11
winebarrel wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the project documentation with a Japanese-language overview of TenkiMap and makes small Swift refactors (notably around Sendable conformance) plus a tweak to AMeDAS icon selection logic.
Changes:
- Expanded
README.mdwith a Japanese description, feature list, and data sources. - Removed
Sendableconformances from several app model types and did minor formatting refactors. - Adjusted AMeDAS weather icon selection logic (nighttime behavior) and did minor readability formatting.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| TenkiMap/WeatherMapViewModel.swift | Removes Sendable from MapMode and minor formatting tweaks. |
| TenkiMap/WeatherItem.swift | Removes Sendable, reformats temperatureColor, and updates AMeDAS icon logic. |
| TenkiMap/JMAService.swift | Removes Sendable from forecast-related types and applies formatting changes; still uses task groups. |
| TenkiMap/ContentView.swift | Reorders imports. |
| TenkiMap/AreaHierarchyService.swift | Removes Sendable from Municipality. |
| TenkiMap/AMEDASService.swift | Removes Sendable from AMeDAS data types. |
| README.md | Adds Japanese documentation (overview/features/data sources). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| struct Forecast { | ||
| let weatherCode: String | ||
| let weather: String? | ||
| let tempMin: String? | ||
| let tempMax: String? | ||
| let pop: String? | ||
| } |
There was a problem hiding this comment.
Forecast no longer conforms to Sendable, but it is used inside TaskResult: Sendable (see fetchForecasts), which requires all stored property types to be Sendable. As-is, this will fail to compile. Re-add Sendable conformance to Forecast (and any other nested types used in the task-group result) or remove/adjust the Sendable requirement in the task-group plumbing (only use @unchecked Sendable with a clear safety rationale).
| group.addTask { | ||
| let result = await fetchAndParse(apiCode: apiCode, offices: offices) | ||
| return TaskResult(timeDefines: result.timeDefines, | ||
| officePairs: result.officePairs, areaPairs: result.areaPairs) | ||
| officePairs: result.officePairs, areaPairs: result.areaPairs) | ||
| } |
There was a problem hiding this comment.
group.addTask { ... } uses an @Sendable task closure, and the captured offices value is [JMAOffice]. Since JMAOffice no longer conforms to Sendable, this will trigger Sendable-checking errors/warnings (and can be an error under strict concurrency). If you keep the task group, please restore Sendable on JMAOffice (it should be safe given it only stores value types), or refactor to avoid passing non-Sendable values into child tasks.
| into tempsByAreaDay: inout TempsByAreaDay) | ||
| { |
There was a problem hiding this comment.
parseShortTermTemps is formatted with the closing ) and opening { split across lines. This makes the signature harder to read and is inconsistent with most of the codebase’s function declaration formatting. Consider either keeping ) and { on the same line, or fully formatting the signature/brace in a consistent multi-line style (but avoid the mixed style).
| into tempsByAreaDay: inout TempsByAreaDay) | |
| { | |
| into tempsByAreaDay: inout TempsByAreaDay) { |
Added images to the README for visual representation.
This pull request primarily updates the
README.mdfile to add a detailed Japanese-language description of the app and its features, and makes minor code changes to theWeatherItem.swiftfile to adjust protocol conformance and icon display logic.Documentation improvements:
README.md, including details on weather data sources and app functionality.Codebase adjustments:
Sendableprotocol conformance fromWeatherSourceandWeatherItemenums and structs inWeatherItem.swift, likely for compatibility or simplification. [1] [2]temperatureColorproperty inWeatherItemto use a multi-line format for clarity.WeatherItemso that during nighttime hours, both sunny and cloudy weather now display the "sun.max.fill" icon instead of "moon.stars.fill".