Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Little refactors #17

Merged
merged 4 commits into from
Nov 27, 2024
Merged

Little refactors #17

merged 4 commits into from
Nov 27, 2024

Conversation

PaulRBerg
Copy link
Member

No description provided.

@PaulRBerg PaulRBerg force-pushed the refactor/prb-feedback branch from 32e61aa to 718074c Compare November 26, 2024 19:14
@PaulRBerg
Copy link
Member Author

Not sure why the CI fails, the build works locally, @sablier-labs/solidity PTAL

@smol-ninja
Copy link
Member

The build fails because the create function in lockup now requires shape input. So any PR in this repo depends on a PR that resolves this issue. I am just testing something then will create a PR to resolve that issue.

@PaulRBerg
Copy link
Member Author

Cheers

smol-ninja
smol-ninja approved these changes Nov 26, 2024
View reviewed changes
Copy link
Member

@smol-ninja smol-ninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Left a few comments below.

remappings.txt Outdated Show resolved Hide resolved
src/SablierMerkleLL.sol Outdated Show resolved Hide resolved
andreivladbrg
andreivladbrg approved these changes Nov 27, 2024
View reviewed changes
Copy link
Member

@andreivladbrg andreivladbrg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a terminology question

src/interfaces/ISablierMerkleBase.sol Show resolved Hide resolved
@andreivladbrg
Copy link
Member

andreivladbrg commented Nov 27, 2024 •
edited
Loading

@razgraf tagging you as this PR changes the variables from merkle campaigns from CANCELABLE/TRANSFERABLE to STREAM_CANCELABLE/STREAM_TRANSFERABLE as it may be relevant to the subgraph

PaulRBerg and others added 2 commits November 27, 2024 18:29
@PaulRBerg @andreivladbrg
refactor: add STREAM_ prefix
b4004e7 
docs: update NatSpec
refactor: import from src
@smol-ninja @andreivladbrg
chore: removes redundant remapping
5f9b12d 
perf: marks _schedule as private
@smol-ninja smol-ninja force-pushed the refactor/prb-feedback branch from 5f9b12d to fe87ad1 Compare November 27, 2024 16:32
@smol-ninja
Copy link
Member

@andreivladbrg we can merge this PR now. Added f1d6a25 which removes some redundant whitespaces and orders variable in tests in alphabetical order.

@andreivladbrg andreivladbrg merged commit b0f2938 into main Nov 27, 2024
7 checks passed
@andreivladbrg andreivladbrg deleted the refactor/prb-feedback branch November 27, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smol-ninja smol-ninja smol-ninja approved these changes

@andreivladbrg andreivladbrg andreivladbrg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PaulRBerg @smol-ninja @andreivladbrg