Skip to content

add 'coverage' dev-dependency and update CONTRIBUTING.md #9

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.

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

stevengum
Copy link
Member

@stevengum stevengum commented Dec 8, 2024

Example code coverage report in html format:
image

@stevengum stevengum requested a review from a team as a code owner December 8, 2024 04:12
Copy link
Contributor

@corinagum corinagum left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -9,6 +9,7 @@ dependencies = ["agent-runtime", "memory-module", "litellm"]
[tool.uv]
package = false
dev-dependencies = [
"coverage>=7.6.9",
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if coverage details are that important at this stage of the project. But feel free to include it and have it run as a github action.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with your point, but I think it's a good for two scenarios:

  1. setting up a quality bar for the team to enforce later on (I say later on because it's important for the team to move quickly now)
  2. people who aren't part of the core team can contribute via tests and learn the codebase

Copy link
Contributor

@heyitsaamir heyitsaamir Jan 13, 2025

Choose a reason for hiding this comment

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

Hey @stevengum, if you still have bandwidth, could you rebase this? We'd love to merge this in

@rajan-chari
Copy link
Contributor

Some failed tests?

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.

4 participants