Skip to content
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

[Issue 740] current sprint report #760

Merged

Conversation

widal001
Copy link
Collaborator

@widal001 widal001 commented Nov 24, 2023

Summary

Adds support for calculating sprint burndown using @current instead of specifying the name of a sprint. This allows us to set a schedule-based GitHub action that will calculate sprint burndown dynamically.

Fixes #740

Time to review: 5 mins

Changes proposed

What was added, updated, or removed in this PR.

  • Adds two new methods to SprintBoard
    • SprintBoard.get_sprint_name_from_date() Which accepts a date and returns the name of the sprint that contains it.
    • SprintBoard.current_sprint Which returns the name of the currently active sprint
  • Adds private method SprintBurndown._get_and_validate_sprint_name() which is used to ensure that the value passed to the sprint argument is a valid sprint name
  • Refactors BaseMetric.chart to be a property instead of an attribute whose value was set during __init__() This change defers the invocation of plot_results() which is a slow method and, as a result, it speeds up initialization overall.
  • Updates README to demonstrate use of @current

Context for reviewers

Testing instructions, background context, more in-depth details of the implementation, and anything else you'd like to call out or ask reviewers. Explain how the changes were verified.

  1. Checkout the PR locally: git fetch origin && git checkout issue-740-current-sprint-report
  2. Follow the installation steps in the analytics/README.md file
  3. Test the new @current functionality with the following commands:
poetry run analytics export gh_issue_data --owner HHS --repo simpler-grants-gov --output-file data/issue-data.json
poetry run analytics export gh_project_data --owner HHS --project 13 --output-file data/sprint-data.json
poetry run analytics calculate sprint_burndown --sprint-file data/sprint-data.json --issue-file data/issue-data.json --sprint @current --show-results

Additional information

Screenshots, GIF demos, code examples or output to help show the changes working as expected.

Setting the BaseMetric.chart attribute during the __init__() method was slow
By deferring the execution of plot_results() until BaseMetric.chart is called
we can speed up the __init__() method
This property returns the name of the current sprint, which depends on
another new method: get_sprint_name_from_date()
Allows users to pass "@current" as the sprint to calculate burndown
for the current sprint
This reduces duplication across tests for creating sample SprintBoard data
Copy link
Collaborator

@lucasmbrown-usds lucasmbrown-usds left a comment

Choose a reason for hiding this comment

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

Looks great! Nice work!

# fmt: on
matching_sprints = self.sprints.loc[date_filter, self.sprint_col]
# if there aren't any sprints return None
if len(matching_sprints) == 0:
Copy link
Collaborator

Choose a reason for hiding this comment

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

if len > 1, should we raise an error rather than silently choosing the first one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh good point! I'll add a quick fix for this before merging

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually I'm working through a fairly complicated merge conflict upstream, to simplify things I might spike getting this into #685 so I'm just resolving one merge conflict.

analytics/src/analytics/metrics/base.py Show resolved Hide resolved
analytics/src/analytics/metrics/base.py Show resolved Hide resolved
analytics/src/analytics/metrics/burndown.py Show resolved Hide resolved
@widal001 widal001 merged commit 892946f into issue-314-30k-deliverable-reporting Nov 28, 2023
2 checks passed
@widal001 widal001 deleted the issue-740-current-sprint-report branch November 28, 2023 19:28
widal001 added a commit that referenced this pull request Nov 28, 2023
Enables users to calculate sprint burndown for the current sprint
by passing `@current` to the `sprint` argument
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.

2 participants