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 743] Sprint burndown by points #782

Merged
merged 9 commits into from
Dec 1, 2023

Conversation

widal001
Copy link
Collaborator

@widal001 widal001 commented Nov 30, 2023

Summary

Adds support for calculating sprint burndown by points instead of tickets, and fixes a few other requested changes that @lucasmbrown-usds made in #685

Fixes #743

Time to review: 10 mins

Changes proposed

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

  • Modifies SprintBurndown to accept a unit parameter which uses the same Unit enum as DeliverablePercentComplete
  • Updates SprintBurndown.calculate() to use the unit parameter when calculating burndown, allowing us to calculate burndown by sprint and tickets
  • Extracts the max number of records returned by the GitHub etl functions and increases it to 10,000 while adding a note to switch to an incremental export and load pattern
  • Adds the units to the chart title, axis, and slack post for SprintBurndown
  • Refactors our use of Unit in DeliverablePercentComplete to avoid using the hardcoding "tasks" or "points" as a string outside of the enum
  • Updates the README to provide guidance on setting the slack secrets

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: git fetch origin && git checkout issue-743-sprint-burndown-by-points
  2. Follow the "Getting started" instructions if you don't have it set up locally
  3. Run: poetry run analytics calculate sprint_burndown --sprint-file data/sprint-data.json --issue-file data/issue-data.json --sprint "Sprint 10" --unit points --show-results

Open questions

  • Do we want to change the unit tasks to tickets?

Additional information

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

Screenshot 2023-11-30 at 9 44 01 AM Screenshot 2023-11-30 at 9 43 40 AM

The code copied from deliverable percent complete copied the dataframe
from self.dataset instead of using the filtered dataset passed to the
method. This introduced a bug that this commit fixes.
Also adds a TODO for setting up incremental exports
@widal001 widal001 marked this pull request as draft November 30, 2023 14:45
Replaces references to "points" and "tasks" string with Enum in
DeliverablePercentComplete metric class
@widal001 widal001 marked this pull request as ready for review November 30, 2023 18:19
@EOKENAVA EOKENAVA self-requested a review November 30, 2023 20:36
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.

Overall looks excellent. On the question of referring to things as either "tasks" or "stories" or something else, I wonder if it makes sense to use the same language as GitHub for consistency and clarity, and call them "issues"?

But I don't have too strong opinions about this, either way is fine for me.

Nice work!

analytics/README.md Show resolved Hide resolved
analytics/README.md Outdated Show resolved Hide resolved
analytics/src/analytics/cli.py Show resolved Hide resolved
analytics/src/analytics/etl/github.py Show resolved Hide resolved
analytics/src/analytics/metrics/percent_complete.py Outdated Show resolved Hide resolved
analytics/tests/metrics/test_burndown.py Show resolved Hide resolved
This helps align terminology in the project with terminology used in GitHub
Storing the value of self.unit.value to the variable unit is confusing
because it doesn't indicate the type or how that variable will be used
This commit changes the value to unit_col to indicate that it will be
used to select the unit column and adds a type hint
@widal001 widal001 merged commit c8b6e12 into main Dec 1, 2023
2 checks passed
@widal001 widal001 deleted the issue-743-sprint-burndown-by-points branch December 1, 2023 16:54
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.

[Task]: Analytics - Add support for sprint burndown by points
2 participants