Skip to content

Conversation

@gusinacio
Copy link
Contributor

@gusinacio gusinacio commented Feb 7, 2025

  • Simply filling the gaps that was missing with unimplemented!()
  • Using rstest to provide different fixtures with different contexts to the same test

@gusinacio gusinacio force-pushed the gustavo/impl-tap-manager-traits branch 3 times, most recently from f779ee3 to b76373c Compare February 7, 2025 17:21
@coveralls
Copy link

coveralls commented Feb 7, 2025

Pull Request Test Coverage Report for Build 13246907631

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 434 of 466 (93.13%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.6%) to 79.353%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/tap-agent/src/tap/context/receipt.rs 343 375 91.47%
Totals Coverage Status
Change from base Build 13243601845: 0.6%
Covered Lines: 7702
Relevant Lines: 9706

💛 - Coveralls

carlosvdr
carlosvdr previously approved these changes Feb 7, 2025
Copy link
Contributor

@carlosvdr carlosvdr left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@suchapalaver suchapalaver left a comment

Choose a reason for hiding this comment

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

@gusinacio thanks! Let me know if you want to chat about the trait that looks like a static function. I wonder if breaking this PR up into its increments might help get all the intentions 🙏

@gusinacio gusinacio enabled auto-merge (squash) February 10, 2025 19:16
@gusinacio gusinacio merged commit d08b3e3 into main Feb 10, 2025
10 checks passed
@gusinacio gusinacio deleted the gustavo/impl-tap-manager-traits branch February 10, 2025 20:01
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.

5 participants