-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: release v0.8.0 #189
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe pull request includes updates to the Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
d1b5600
to
5dbd1b3
Compare
5dbd1b3
to
9804604
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
crates/augurs-prophet/CHANGELOG.md (1)
14-14
: Consider documenting the impact of the Forecaster wrapperWhile the addition of the Forecaster wrapper is noted, it would be helpful to briefly describe its purpose and impact.
Consider expanding the changelog entry to include a brief description:
- add Forecaster wrapper for Prophet (#191) + add Forecaster wrapper for Prophet to provide a unified forecasting interface (#191)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
Cargo.toml
(2 hunks)crates/augurs-forecaster/CHANGELOG.md
(1 hunks)crates/augurs-prophet/CHANGELOG.md
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/augurs-forecaster/CHANGELOG.md
🔇 Additional comments (4)
Cargo.toml (2)
21-21
: LGTM: Workspace package version updated to 0.8.0
The workspace package version has been correctly updated to 0.8.0.
32-41
: Verify breaking changes in augurs-forecaster
While the version updates are consistent across all packages, the PR objectives mention breaking changes in augurs-forecaster due to new enum variants. This warrants special attention for downstream dependencies.
Let's check for any dependent packages that might be affected:
crates/augurs-prophet/CHANGELOG.md (2)
10-22
: LGTM: Well-structured changelog entry for 0.8.0
The changelog entry follows the proper format and includes all necessary sections with appropriate PR references.
22-22
: Verify compatibility with updated statrs dependency
The update of statrs from 0.17.1 to 0.18.0 should be verified for any breaking changes.
Let's check for any potential breaking changes:
✅ Verification successful
Dependency update appears safe to proceed
The codebase uses only basic distribution constructors (new()
) and sampling methods from statrs, specifically for Normal, Poisson, and Laplace distributions. These are fundamental statistical distributions with stable APIs that are unlikely to have breaking changes. The usage is straightforward and defensive with proper error handling through expect()
. The update from 0.17.1 to 0.18.0 is a minor version bump, which should maintain backward compatibility according to semver.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for statrs usage in the codebase
rg "use.*statrs|from.*statrs"
# Look for any potential breaking changes in statrs 0.18.0
gh api repos/statrs-dev/statrs/releases/tags/v0.18.0 --jq .body
Length of output: 354
Script:
#!/bin/bash
# Let's check the actual usage of statrs in the code
rg -A 5 "statrs" crates/augurs-prophet/src/prophet/predict.rs
# Let's also check the Cargo.toml to see the dependency specification
cat crates/augurs-prophet/Cargo.toml
Length of output: 3075
Script:
#!/bin/bash
# Let's check the actual implementation to see how statrs distributions are used
rg -A 10 "Laplace|Normal|Poisson" crates/augurs-prophet/src/prophet/predict.rs
# Let's also check git history for the specific change
git show --stat $(git log --format=%H -1 --grep="#187")
Length of output: 2124
adde9d2
to
2d9c11b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
crates/augurs-forecaster/CHANGELOG.md (1)
21-21
: Consider moving power transformation entry to Added section.The addition of power transformation logic appears to be a new feature and would be more appropriate under the "Added" section rather than "Other".
Consider moving this entry to the "Added" section to better reflect the nature of the change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Cargo.toml
(2 hunks)crates/augurs-forecaster/CHANGELOG.md
(1 hunks)crates/augurs-prophet/CHANGELOG.md
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Cargo.toml
- crates/augurs-prophet/CHANGELOG.md
🔇 Additional comments (1)
crates/augurs-forecaster/CHANGELOG.md (1)
9-10
: LGTM: Version header and comparison link are correctly formatted.
The version header follows the Keep a Changelog format and includes a proper comparison link.
### Added | ||
|
||
- add 'transforms' JS crate and include in augurs JS bindings (#195) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Document breaking changes in the Added section.
The PR objectives mention that augurs-forecaster has breaking changes due to a non-exhaustive enum. This should be clearly marked as breaking in the changelog.
Apply this diff to clearly indicate the breaking change:
### Added
- - add 'transforms' JS crate and include in augurs JS bindings (#195)
+ - [**breaking**] add 'transforms' JS crate and include in augurs JS bindings (#195)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
### Added | |
- add 'transforms' JS crate and include in augurs JS bindings (#195) | |
### Added | |
- [**breaking**] add 'transforms' JS crate and include in augurs JS bindings (#195) | |
|
||
### Fixed | ||
|
||
- make Transform enum non-exhaustive (#194) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Document impact of non-exhaustive Transform enum.
The change to make Transform enum non-exhaustive is a breaking change that requires pattern matching to include a wildcard pattern. This impact should be documented to help users update their code.
Apply this diff to add migration instructions:
- - make Transform enum non-exhaustive (#194)
+ - [**breaking**] make Transform enum non-exhaustive (#194) - requires adding a wildcard pattern (`_`) to Transform enum matches
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- make Transform enum non-exhaustive (#194) | |
- [**breaking**] make Transform enum non-exhaustive (#194) - requires adding a wildcard pattern (`_`) to Transform enum matches |
2d9c11b
to
92423b5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Cargo.toml
(2 hunks)crates/augurs-forecaster/CHANGELOG.md
(1 hunks)crates/augurs-prophet/CHANGELOG.md
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Cargo.toml
- crates/augurs-prophet/CHANGELOG.md
🔇 Additional comments (3)
crates/augurs-forecaster/CHANGELOG.md (3)
9-10
: LGTM: Version entry follows the Keep a Changelog format.
The version entry and GitHub comparison link are correctly formatted.
11-14
: Document breaking changes in the Added section.
The PR objectives mention that augurs-forecaster has breaking changes. This should be clearly marked as breaking in the changelog.
15-17
: Document impact of non-exhaustive Transform enum.
The change to make Transform enum non-exhaustive is a breaking change that requires pattern matching to include a wildcard pattern. This impact should be documented to help users update their code.
### Other | ||
|
||
- precalculate offset and scale factor for min-max scale transformer (#196) | ||
- Add power transformation logic to forecaster transforms ([#185](https://github.com/grafana/augurs/pull/185)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve changelog organization by properly categorizing changes.
The entries in the "Other" section appear to be functional changes that should be categorized more specifically:
- "precalculate offset..." is a performance enhancement that should go under "Changed" or "Fixed"
- "Add power transformation..." is a new feature that should go under "Added"
Apply this diff to reorganize the changes:
### Added
- add 'transforms' JS crate and include in augurs JS bindings (#195)
+- Add power transformation logic to forecaster transforms ([#185](https://github.com/grafana/augurs/pull/185))
-### Fixed
+### Changed
- make Transform enum non-exhaustive (#194)
+- precalculate offset and scale factor for min-max scale transformer (#196)
-
-### Other
-
-- precalculate offset and scale factor for min-max scale transformer (#196)
-- Add power transformation logic to forecaster transforms ([#185](https://github.com/grafana/augurs/pull/185))
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
### Other | |
- precalculate offset and scale factor for min-max scale transformer (#196) | |
- Add power transformation logic to forecaster transforms ([#185](https://github.com/grafana/augurs/pull/185)) | |
### Added | |
- add 'transforms' JS crate and include in augurs JS bindings (#195) | |
- Add power transformation logic to forecaster transforms ([#185](https://github.com/grafana/augurs/pull/185)) | |
### Changed | |
- make Transform enum non-exhaustive (#194) | |
- precalculate offset and scale factor for min-max scale transformer (#196) |
Published a canary version of the JS package as |
2314e0d
to
1399b96
Compare
1399b96
to
a421dcd
Compare
🤖 New release
augurs
: 0.7.0 -> 0.8.0augurs-changepoint
: 0.7.0 -> 0.8.0augurs-core
: 0.7.0 -> 0.8.0augurs-clustering
: 0.7.0 -> 0.8.0augurs-dtw
: 0.7.0 -> 0.8.0augurs-ets
: 0.7.0 -> 0.8.0augurs-mstl
: 0.7.0 -> 0.8.0augurs-forecaster
: 0.7.0 -> 0.8.0 (augurs-outlier
: 0.7.0 -> 0.8.0augurs-prophet
: 0.7.0 -> 0.8.0 (✓ API compatible changes)augurs-seasons
: 0.7.0 -> 0.8.0augurs-forecaster
breaking changesChangelog
augurs
augurs-changepoint
augurs-core
augurs-clustering
augurs-dtw
augurs-ets
augurs-mstl
augurs-forecaster
augurs-outlier
augurs-prophet
augurs-seasons
This PR was generated with release-plz.
Summary by CodeRabbit
New Features
Bug Fixes
chrono
library.Documentation