Conversation
Removed unused comment.
Completely removed anything NAPI related from this repository.
Made a potential panic propagate instead using the `?` operator
Centralized GET method request logic to reduce code size by a small margin
BREAKING CHANGE: `NightscoutClient.entries().sgv()` and `NightscoutClient.entries().mbg()` are now accessible directly via `NightscoutClient.mbg()` and `NightscoutClient.sgv()`
There was a problem hiding this comment.
Pull request overview
This PR appears to prepare the crate for a 1.0.0 release by removing the Node/N-API bindings and consolidating HTTP GET + JSON parsing logic into a reusable client helper, simplifying several service implementations.
Changes:
- Removed N-API/Node bindings and associated build/package artifacts (Rust
cdylib,build.rs, npmpackage*.json). - Refactored the Rust client API (removed
entries()service wrapper; addedsgv()/mbg()and a genericNightscoutClient::fetch<T>()helper). - Updated multiple services/models to drop
napiannotations and to use the newfetch<T>()helper for GETs.
Reviewed changes
Copilot reviewed 14 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/client.rs | Adds fetch<T>() and changes public API surface (sgv()/mbg()), but currently has a critical Result type issue. |
| src/query_builder.rs | Uses client.fetch() to simplify GET paths, but currently has a critical Result type issue. |
| src/models/status.rs | Switches Status GET logic to client.fetch() and removes napi annotations. |
| src/models/properties.rs | Switches Properties GET logic to client.fetch() and removes napi annotations. |
| src/models/profile.rs | Switches Profile GET logic to client.fetch() and removes napi annotations. |
| src/models/entries.rs | Removes EntriesService wrapper, updates URL building, removes napi annotations; contains outdated doc example. |
| src/models/devicestatus.rs | Removes napi annotations. |
| src/models/treatments.rs | Removes napi annotations. |
| src/models/trends.rs | Removes napi string enum annotation/import. |
| src/lib.rs | Stops exporting node_bindings. |
| src/node_bindings.rs | Deletes Node binding layer. |
| Cargo.toml / Cargo.lock | Removes napi* deps/build config, updates dependency versions, switches crate-type to rlib. |
| build.rs | Removes napi-build setup script. |
| package.json / package-lock.json | Removes npm packaging/build metadata. |
| .gitignore | Removes N-API build artifact ignore entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub async fn create(&self, entries: Vec<SgvEntry>) -> Result<Vec<SgvEntry>, NightscoutError> { | ||
| let url = self | ||
| .client | ||
| .base_url | ||
| .join(Endpoint::Entries.as_path()) | ||
| .expect("URL Error"); | ||
| .join(Endpoint::Entries.as_path())?; | ||
|
|
There was a problem hiding this comment.
The SgvService::list doc example above is now out of date and likely fails doctests: it uses the removed client.entries().sgv() API (client now exposes sgv()/mbg() directly), it calls NightscoutClient::new(URL, SECRET) even though new takes Option<String> for the secret, and it references Duration without importing it in the snippet. Update the example to match the current public API so the docs compile.
| categories = ["api-bindings", "asynchronous", "data-structures", "science::bioinformatics", "web-programming::http-client"] | ||
| build = "build.rs" | ||
|
|
||
| [lib] | ||
| crate-type = ["cdylib", "rlib"] | ||
| crate-type = ["rlib"] | ||
|
|
There was a problem hiding this comment.
PR title/description indicate a 1.0.0 release, but Cargo.toml still declares version = "0.1.3". If this PR is intended to ship 1.0.0, the crate version should be bumped accordingly (or clarify that versioning will happen in a separate PR).
src/query_builder.rs
Outdated
| Method::GET => { | ||
| let mut request = self.client.http.get(url); | ||
| request = self.client.auth(request); | ||
|
|
||
| let response = self.client.send_checked(request).await?; | ||
| let items = response.json::<Vec<T>>().await?; | ||
| let items: Vec<T> = self.client.fetch(url).await?; | ||
|
|
There was a problem hiding this comment.
This file still imports anyhow::Result (type alias Result<T>), but the IntoFuture impl uses Result<Vec<T>, NightscoutError> elsewhere in the file. That combination does not compile (type alias takes 1 generic argument, 2 supplied). Remove the anyhow::Result import and use std::result::Result (or define a local alias that includes NightscoutError).
| use super::error::NightscoutError; | ||
|
|
||
| use anyhow::Result; | ||
| use reqwest::{Client as HttpClient, Response}; | ||
| use sha1::{Digest, Sha1}; | ||
| use url::Url; |
There was a problem hiding this comment.
use anyhow::Result; makes Result a 1-parameter alias (Result<T>) but this file uses Result<T, NightscoutError> in multiple signatures (e.g., new, send_checked, fetch), which will not compile. Remove the anyhow::Result import and use std::result::Result (or define a local type Result<T> = std::result::Result<T, NightscoutError>). If anyhow is no longer used after that, consider removing it from Cargo.toml.
BREAKING CHANGE: `NightscoutClient.new()` now only takes `base_url` and takes an optional secret with the `.with_secret(secret)` method
Fixed client-side device filtering by pushing it to the server-side. Additionally, fixed an issue where device filtering wouldn't work when deleting data. Now using `.device()` it will also be able to filter when deleting nightscout data.
Wrapped the NightscoutClient iniside an Arc<T> to improve cloning performance. Additionally removed the unused api_secret field.
BREAKING CHANGE: .list() now becomes .get() and to send a request you must do .send().await?; instead of simply .await?; This more verbose syntax is clearer and unified with the rest of the codebase.
Anyhow is not intended to be used with libraries as it obscures errors.
BREAKING CHANGE: Ok I know this is dumb putting this in a docs commit but whatever. changed the last .list() -> .get() Added docstrings for docs.rs and a docs.rs homepage Removed unused JS/TS examples
Added tests and fixed docstrings. Also bumped up versions of some crates.
This PR will include all changes for the 1.0.0 update
Check the 1.0.0 milestone for more info