Skip to content

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

Open
stevengum wants to merge 4 commits intomicrosoft:mainfrom
stevengum:stgum/coverage
Open

add 'coverage' dev-dependency and update CONTRIBUTING.md#9
stevengum wants to merge 4 commits intomicrosoft:mainfrom
stevengum:stgum/coverage

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
Collaborator

@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

[tool.uv]
package = false
dev-dependencies = [
"coverage>=7.6.9",
Copy link
Collaborator

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
Collaborator

@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