Skip to content

Conversation

@taslimmuhammed
Copy link
Contributor

Fixes #364
Hi, please have a look when you're free, and as usual tell me if any changes has to be made

@coveralls
Copy link

coveralls commented Oct 23, 2024

Pull Request Test Coverage Report for Build 11483643250

Details

  • 29 of 64 (45.31%) changed or added relevant lines in 6 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 71.791%

Changes Missing Coverage Covered Lines Changed/Added Lines %
common/src/tap.rs 0 1 0.0%
tap-agent/src/agent/sender_account.rs 15 16 93.75%
common/src/tap/checks/allocation_eligible.rs 0 3 0.0%
tap-agent/src/agent/sender_accounts_manager.rs 8 18 44.44%
common/src/allocations/monitor.rs 0 20 0.0%
Files with Coverage Reduction New Missed Lines %
common/src/allocations/monitor.rs 1 0.0%
Totals Coverage Status
Change from base Build 11463323294: -0.2%
Covered Lines: 4805
Relevant Lines: 6693

💛 - Coveralls

Copy link
Contributor

@gusinacio gusinacio left a comment

Choose a reason for hiding this comment

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

You are getting really good on this. Just some really small things!

If typos and some unnecessary map_errs is the best I could find, you are crushing it.

sleep(interval.div_f32(2.0))
},
)
.map_err(|e| e.to_string());
Copy link
Contributor

Choose a reason for hiding this comment

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

You dont need to map to strings here.

#[tokio::test]
async fn test_attestation_signers_update_with_allocations() {
let (mut allocations_writer, allocations) = Eventual::<HashMap<Address, Allocation>>::new();
let (allocations_tx, allcoations_rx) = watch::channel(HashMap::new());
Copy link
Contributor

Choose a reason for hiding this comment

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

typo allocations_rx

@taslimmuhammed
Copy link
Contributor Author

thanks for the review[✌️], I'll send updates asap.

@taslimmuhammed
Copy link
Contributor Author

please have a look, BTW do you know review the code just by looking at changes on github or is there any other way🤓

Copy link
Contributor

@gusinacio gusinacio left a comment

Choose a reason for hiding this comment

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

LGTM, you are doing incredible!

@gusinacio gusinacio changed the title Drop eventuals in favor of Tokio watch + timers for allocations refactor: use tokio::watch for allocations Oct 23, 2024
@gusinacio gusinacio merged commit 8e827ee into graphprotocol:main Oct 23, 2024
7 of 10 checks passed
@taslimmuhammed
Copy link
Contributor Author

Thanks again sir🫡

@taslimmuhammed
Copy link
Contributor Author

taslimmuhammed commented Oct 23, 2024

BTW I don't know if it's right time to ask, but is there any way i could be part of the team? as an intern or junior or any would be great, I really love working on this project

@gusinacio
Copy link
Contributor

BTW I don't know if it's right time to ask, but is there any way i could be part of the team? as an intern or junior or any would be great, I really love working on this project

Hey, you could check here for jobs within Graph Protocol.

I work with Semiotic Labs and while we don't have any position open right now, I suggest you sending your resume here.

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.

Drop eventuals in favor of Tokio watch + timers for allocations

3 participants