Conversation
docs: add new versions in CHANGELOG files
9cab47d to
6d0010f
Compare
flow/CHANGELOG.md
Outdated
|
|
||
| - Remove `Recover` event from `ISablierFlow` ([#1439](https://github.com/sablier-labs/lockup/pull/1439)) | ||
| - Remove surplus zero check from `recover` function ([#1439](https://github.com/sablier-labs/lockup/pull/1439)) | ||
| - Remove `SablierFlow_SurplusZero` error ([#1439](https://github.com/sablier-labs/lockup/pull/1439)) |
There was a problem hiding this comment.
i think we can merge these 3 into a single point, wdyt? @smol-ninja
There was a problem hiding this comment.
I don't think we need to include error point in changelog. If zero surplus check is removed from recover function that inherently means the error is removed too.
Also the first one "Remove Recover event from ISablierFlow" is a breaking change as hypothetically (since its only used internally) it breaks indexers from tracking recover.
There was a problem hiding this comment.
Also, since its a breaking change, shouldn't Flow version be 3.0.0?
7dbe5b4 to
934f044
Compare
934f044 to
45f1bee
Compare
WalkthroughThis pull request updates version numbers and changelog entries across multiple packages in the monorepo. The airdrops package is bumped from 2.0.1 to 3.0.0, flow from 2.0.1 to 2.1.0, lockup from 3.0.1 to 4.0.0, and utils from 1.0.2 to 1.1.0. Each package.json file reflects its corresponding version increment. Changelog files document the release notes (refactorings, added/removed items, and breaking changes). Tests that compute CREATE2 salts are updated to use the new version strings. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
smol-ninja
left a comment
There was a problem hiding this comment.
I created a PR for my suggestions as I found that better than commenting here (since they are many):
| "dependencies": { | ||
| "@openzeppelin/contracts": "5.3.0", | ||
| "@prb/math": "4.1.0", | ||
| "@sablier/evm-utils": "file:../utils", |
There was a problem hiding this comment.
Note that release should be in the following order so that we can update these deps while deploying
- utils
- lockup
- airdrop / flow / bob
This means we should merge everything into main first and update these deps as we start deploying.
|
|
||
| ### Changed | ||
|
|
||
| - Refactor `SablierMerkleBase` and `SablierMerkleLockup` constructors to accept `ConstructorParams` struct ([#1403](https://github.com/sablier-labs/lockup/pull/1403)) |
There was a problem hiding this comment.
How is this relevant to changelog? IMO changelog should only focus on the changes that affect users and since this is an interface refactor, we should skip this.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@airdrops/CHANGELOG.md`:
- Around line 11-13: Update the changelog wording to state that DataTypes is
already deprecated and only retained for backward compatibility: change the
sentence about `DataTypes` (referring to `airdrops/src/types/DataTypes.sol` and
the `DataTypes` identifier) from "will be deprecated in future versions" to
something like "is deprecated and retained for backward compatibility." Keep the
rest of the bullet intact and ensure the deprecation wording aligns with the
actual `DataTypes.sol` notice.
In `@flow/CHANGELOG.md`:
- Around line 7-21: The changelog lists breaking changes (renaming Helpers ->
FlowHelpers and removing Recover event and SablierFlow_SurplusZero from
ISablierFlow and recover) but uses a minor version; update this entry to the
next major version (e.g., bump 2.1.0 -> 3.0.0) and adjust the compare/PR links
to point to the major release tag/compare, and clearly mark the items as
breaking changes (mention Helpers, FlowHelpers, Recover event,
SablierFlow_SurplusZero, ISablierFlow, and transferFromPayable) so downstream
consumers see this is a major/ABI-breaking release.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2bc687b7-a169-4ff9-a40d-e6592bca3d0b
📒 Files selected for processing (12)
airdrops/CHANGELOG.mdairdrops/package.jsonairdrops/tests/utils/BaseScript.t.solflow/CHANGELOG.mdflow/package.jsonflow/tests/utils/BaseScript.t.sollockup/CHANGELOG.mdlockup/package.jsonlockup/tests/utils/BaseScript.t.solutils/CHANGELOG.mdutils/package.jsonutils/tests/integration/fuzz/base-script/BaseScript.t.sol
🚧 Files skipped from review as they are similar to previous changes (1)
- lockup/CHANGELOG.md
* docs: update changelogs * docs: update deprecate notice * docs: use today's date * docs: bump flow to a major version * docs: fix url in bob changelog * docs: polish changelogs * docs: update changelogs --------- Co-authored-by: Andrei Vlad Birgaoanu <andreivladbrg@gmail.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
flow/CHANGELOG.md (1)
19-19:⚠️ Potential issue | 🟡 MinorClarify event-removal wording to avoid API ambiguity.
At Line 19, “Remove
Recoverevent fromrecoverfunction” is imprecise; events are declared/emitted, not part of a function signature. Consider wording it as removal from the interface/API surface (or emission behavior), whichever is accurate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flow/CHANGELOG.md` at line 19, The changelog line is ambiguous about events vs. function signatures; update the wording at Line 19 to clearly state the change in emission or API surface (e.g., "Stop emitting the `Recover` event from the `recover` function" or "Remove the `Recover` event from the contract's public event interface so `recover` no longer emits it"), choosing the accurate phrasing based on whether the event was removed from emission behavior or the declared interface; ensure the text references the `Recover` event and the `recover` function name exactly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@flow/CHANGELOG.md`:
- Line 19: The changelog line is ambiguous about events vs. function signatures;
update the wording at Line 19 to clearly state the change in emission or API
surface (e.g., "Stop emitting the `Recover` event from the `recover` function"
or "Remove the `Recover` event from the contract's public event interface so
`recover` no longer emits it"), choosing the accurate phrasing based on whether
the event was removed from emission behavior or the declared interface; ensure
the text references the `Recover` event and the `recover` function name exactly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8007347b-4233-4db6-acdb-afd8f725ee05
📒 Files selected for processing (5)
airdrops/CHANGELOG.mdbob/CHANGELOG.mdflow/CHANGELOG.mdlockup/CHANGELOG.mdutils/CHANGELOG.md
✅ Files skipped from review due to trivial changes (1)
- utils/CHANGELOG.md
docs: add new versions in CHANGELOG files