fix: Change float to double as in the update of the spec#44
fix: Change float to double as in the update of the spec#44tomasz-blasz merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates Firebolt C++ interfaces and related test/demo code to align with an updated spec that represents certain numeric fields as double instead of float.
Changes:
- Change
Discovery.watchedprogress parameter fromstd::optional<float>tostd::optional<double>across public API and implementation. - Change
Accessibility::VoiceGuidanceSettings::ratefromfloattodoubleacross public API and JSON parsing. - Update unit/component tests and the API test app demo to use
doubleparsing and JSON extraction.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/unit/discoveryTest.cpp | Updates watched test inputs to std::optional<double>. |
| test/unit/accessibilityTest.cpp | Updates expected JSON extraction for rate to double. |
| test/component/accessibilityTest.cpp | Updates component test JSON extraction for rate to double. |
| test/api_test_app/cpp/discoveryDemo.cpp | Switches console parsing from stof to stod for progress. |
| src/json_types/jsondata_accessibility_types.h | Parses/stores VoiceGuidanceSettings.rate as double. |
| src/discovery_impl.h | Updates DiscoveryImpl::watched signature to std::optional<double>. |
| src/discovery_impl.cpp | Updates DiscoveryImpl::watched definition to std::optional<double>. |
| include/firebolt/discovery.h | Updates public IDiscovery::watched signature to std::optional<double>. |
| include/firebolt/accessibility.h | Updates public VoiceGuidanceSettings.rate field to double. |
Comments suppressed due to low confidence (2)
include/firebolt/accessibility.h:39
- Changing
VoiceGuidanceSettings::ratefromfloattodoublein a public header is an ABI-breaking change (struct layout changes) and a source-breaking change for consumers that take the address of the member or rely on exact type. If this library has versioning/compat requirements, consider a major version bump and/or a backward-compatible approach (e.g., keepfloat ratewith a newdouble rate2, or provide accessors / deprecation path).
bool enabled;
double rate;
bool navigationHints;
include/firebolt/discovery.h:52
- Changing the
IDiscovery::watchedsignature fromstd::optional<float>tostd::optional<double>is a public API/ABI-breaking change. If the project promises ABI stability, this should be accompanied by an appropriate version bump and/or a compatibility layer (e.g., overload retaining thefloatvariant and forwarding to the newdoubleimplementation).
virtual Result<bool> watched(const std::string& entityId, std::optional<double> progress,
std::optional<bool> completed, std::optional<std::string> watchedOn,
std::optional<AgePolicy> agePolicy) const = 0;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1d04d41 to
4804ff1
Compare
4804ff1 to
f15fa08
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
include/firebolt/discovery.h:52
- This changes the public virtual API from
std::optional<float>tostd::optional<double>, which is an ABI- and source-breaking change for any consumers implementing/callingIDiscovery::watched. If this library has compatibility/versioning requirements (soname/major version, release notes, deprecation window), please apply them for this change (e.g., bump ABI/major version and document the breaking change, or provide a transitional overload).
virtual Result<bool> watched(const std::string& entityId, std::optional<double> progress,
std::optional<bool> completed, std::optional<std::string> watchedOn,
std::optional<AgePolicy> agePolicy) const = 0;
include/firebolt/accessibility.h:40
- Changing
VoiceGuidanceSettings::ratefromfloattodoubleis a source/ABI breaking public header change for consumers (struct layout change and any code taking/returning this type). If the project tracks ABI stability, please make sure the appropriate version/ABI bump and release notes are included, or provide a migration path.
struct VoiceGuidanceSettings
{
bool enabled;
double rate;
bool navigationHints;
};
test/unit/discoveryTest.cpp:35
progressis nowdouble, but the test still uses a float literal (0.75f). Consider switching to adoubleliteral (0.75) to avoid an unnecessary float->double conversion and keep the test aligned with the updated API.
std::optional<double> progress = 0.75f;
std::optional<bool> completed = true;
test/unit/discoveryTest.cpp:64
- Same as above:
progressisdoublehere, but the initializer uses a float literal (0.75f). Prefer adoubleliteral (0.75) for consistency with the updated type.
std::optional<double> progress = 0.75f;
std::optional<bool> completed = true;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f15fa08 to
d8e05e9
Compare
|
🎉 This PR is included in version 0.5.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.