Document the GeoLibre R package - #1771
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe documentation adds a complete ChangesR package documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
🔍 Cloudflare PR preview
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| server <- function(input, output, session) { | ||
| map <- reactiveVal(geolibre(map_only = TRUE)) | ||
| output$map <- renderGeolibre(map()) | ||
|
|
||
| observeEvent(input$reset, { | ||
| next_map <- set_view(map(), center = c(0, 20), zoom = 2) | ||
| map(next_map) | ||
| update_geolibre(geolibre_proxy("map"), next_map) | ||
| }) | ||
| } |
There was a problem hiding this comment.
The example contradicts its own explanation. The preceding paragraph says "a proxy updates the live widget without rebuilding the output," but the code has output$map <- renderGeolibre(map()) reactively depend on the map reactiveVal, and then observeEvent calls both map(next_map) (line 128, which invalidates and re-runs renderGeolibre, rebuilding the whole widget) and update_geolibre(geolibre_proxy("map"), next_map) (line 129, the no-rebuild proxy path). Doing both together means the rebuild the proxy is meant to avoid happens anyway, and readers copying this pattern won't get the "no rebuild" behavior the prose promises.
A pattern that actually demonstrates the proxy benefit would drop the reactiveVal/renderGeolibre dependency on next_map and update only via the proxy, e.g.:
server <- function(input, output, session) {
output$map <- renderGeolibre(geolibre(map_only = TRUE))
observeEvent(input$reset, {
update_geolibre(geolibre_proxy("map"), function(m) set_view(m, center = c(0, 20), zoom = 2))
})
}(Exact API may differ — I can't verify the geolibre R package's actual proxy signature since it lives in a separate repo — but the current example as written doesn't achieve what the prose claims.)
🔍 GitHub Pages PR preview
Note GitHub Pages built this preview successfully, but its serving edge returned HTTP 403 when checked. The links may still be propagating. |
Code reviewBugs
Security
Performance
Quality
CLAUDE.md
No other issues found; the diff is a clean, self-consistent documentation addition (new |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
sf, rasters, project files, Shiny, and self-hostingValidation
git diff --checkpassesExisting warnings
MkDocs strict mode remains blocked by two unrelated existing warnings:
server-api.mdis absent from the navigation and the generateddemo/navigation target is not present in the source docs tree.Summary by CodeRabbit
sfand GeoJSON data, remote rasters, camera controls, project persistence, and self-hosting.