- Notifications
You must be signed in to change notification settings - Fork87
fix: remove mentor count from stats if mentors are disabled#519
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Signed-off-by: Zack Koppert <zkoppert@github.com>
… the markdown tableSigned-off-by: Zack Koppert <zkoppert@github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR introduces a newenable_mentor_count flag to conditionally include the mentor count in the markdown output.
- Add
enable_mentor_countparameter throughout writer functions and main invocation - Update tests to remove mentor count when flag is off and pass flag in one test
- Conditionally write the mentor count row only when enabled
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| markdown_writer.py | Addedenable_mentor_count parameter and conditional mentor row |
| issue_metrics.py | Passedenable_mentor_count from main into writer calls |
| test_markdown_writer.py | Removed disabled-mentor-count expectations and set flag in a test |
Comments suppressed due to low confidence (3)
test_markdown_writer.py:370
- There’s no assertion verifying that the mentor count line actually appears when
enable_mentor_count=True. Add something likeassert '| Number of most active mentors |' in markdown_outputto cover the enabled case.
enable_mentor_count=True,issue_metrics.py:284
- The CLI parser isn’t defining an
--enable-mentor-countflag, soenable_mentor_countmay always be unset or cause a NameError. Add an argument likeparser.add_argument('--enable-mentor-count', action='store_true', help='Include mentor count in the report')before it's used.
enable_mentor_count=enable_mentor_count,markdown_writer.py:103
- The new
enable_mentor_countparameter isn’t documented in the function docstring forwrite_to_markdown. Update the docstring to explain its purpose and default behavior.
enable_mentor_count=False,There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actually using the boolean. Nice fix.
0ecfab7 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Pull Request
Proposed Changes
This pull request introduces a new feature to enable tracking and reporting of mentor activity metrics in the issue metrics system. The changes primarily involve adding a new
enable_mentor_countparameter across various functions and updating the logic to conditionally display mentor-related data in the output.Readiness Checklist
Author/Contributor
make lintand fix any issues that you have introducedmake testand ensure you have test coverage for the lines you are introducing@jeffrey-luszczReviewer
fix,documentation,enhancement,infrastructure,maintenance, orbreaking