Skip to content

feat: add new distributor - #377

Open
0xble wants to merge 15 commits into
mainfrom
feat/rooms-distributor
Open

feat: add new distributor#377
0xble wants to merge 15 commits into
mainfrom
feat/rooms-distributor

Conversation

@0xble

@0xble 0xble commented Feb 13, 2024

Copy link
Copy Markdown
Contributor

Motivation

Solution

Link T-4299

@0xble
0xble requested a review from arr00 February 13, 2024 19:55
@height

height Bot commented Feb 13, 2024

Copy link
Copy Markdown

This pull request has been linked to 1 task:

💡Tip: Add "Close T-4299" to the pull request title or description, to a commit message, or in a comment to mark this task as "Done" when the pull request is merged.

@0xble
0xble marked this pull request as draft February 13, 2024 19:55
@codecov-commenter

codecov-commenter commented Feb 13, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 93.61702% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 67.44%. Comparing base (136ed9b) to head (0befa6c).

Files Patch % Lines
contracts/proposals/ProposalExecutionEngine.sol 85.00% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #377      +/-   ##
==========================================
+ Coverage   66.82%   67.44%   +0.61%     
==========================================
  Files          70       71       +1     
  Lines        2963     3010      +47     
  Branches      647      658      +11     
==========================================
+ Hits         1980     2030      +50     
+ Misses        783      779       -4     
- Partials      200      201       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions

github-actions Bot commented Feb 13, 2024

Copy link
Copy Markdown

Gas Report:

Comment thread contracts/distribution/PushDistributor.sol Outdated
@arr00 arr00 mentioned this pull request Feb 13, 2024

@arr00 arr00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Coming along well. Some thoughts so far.

Comment thread contracts/distribution/PushDistributor.sol Outdated
Comment thread contracts/distribution/PushDistributor.sol Outdated
Comment thread contracts/distribution/PushDistributor.sol Outdated
Comment thread contracts/proposals/ProposalExecutionEngine.sol Outdated
arr00 and others added 2 commits February 16, 2024 14:17
@0xble
0xble marked this pull request as ready for review February 16, 2024 19:35

@arr00 arr00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good for the most part. A few comments

Comment thread contracts/distribution/PushDistributor.sol Outdated
Comment thread contracts/distribution/PushDistributor.sol Outdated
Comment thread contracts/distribution/PushDistributor.sol Outdated
Comment thread contracts/distribution/PushDistributor.sol Outdated
Comment thread contracts/distribution/PushDistributor.sol Outdated
Comment thread test/distribution/PushDistributor.t.sol
Comment thread contracts/distribution/PushDistributor.sol
// Token address used to indicate ETH.
IERC20 private constant ETH_ADDRESS = IERC20(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE);

function distribute(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would like detailed natspec here. It's a pretty advanced function. Otherwise PR looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants