Skip to content

Conversation

@hackebrot
Copy link
Contributor

@hackebrot hackebrot commented Mar 12, 2025

This PR introduces the ability to retrieve all commits included in a given deployment, addressing a key requirement for accurately measuring Lead Time for Changes in Four Keys.

Key Changes

  • Added GitHub REST API integration for comparing commits (with support for paginated responses)
  • Rearchitected GitHub code to enable package consumers to select which API to use for higher level operations
  • Added higher level function for querying deployed commits for a given deployment
  • Introduced structured logging to both the metrics CLI app as well as the github package

Recommended review order

  1. go.mod and go.sum for new github.com/google/go-github/v68 dependency
  2. Files under pkg/github/rest/ with new GitHub REST API code
  3. Files under pkg/github/graphql/ with updated GitHub GraphQL API code
  4. Changes for pkg/github/*.go with higher-level GitHub API code
  5. Files under pkg/github/internal/test/ with test framework for pkg/github
  6. Files under metrics/internal/factory/ for refactored factory code
  7. CLI app changes in metrics/cmd/**/*.go
  8. Remaining files in metrics/

Jira ticket

CICD-441

Issues

Resolve #19
Resolve #30
Resolve #33

Notes

Note: AI tools were used in generating this pull request 🤖

@hackebrot hackebrot added feature New feature or request app:metrics Changes related to the metrics CLI app pkg:github Changes related to pkg/github labels Mar 12, 2025
@hackebrot hackebrot requested a review from a team March 12, 2025 13:09
}

if config.commitLimit > 250 {
return fmt.Errorf("commit-limit cannot be greater than 250")
Copy link

Choose a reason for hiding this comment

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

Is there a reason for a 250 commit limit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question! At the time I've added that check, the internal REST API client didn't support pagination yet:
https://github.com/mozilla-services/rapid-release-model/pull/35/files#diff-7525bc78a0e3497bcd41a5862aed2f3e47732af92fe2e4be04f92132aecba748R12-R23

We can probably drop that upper limit.

Copy link

@gdover9 gdover9 Mar 12, 2025

Choose a reason for hiding this comment

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

Cool! 😄

Copy link

@gdover9 gdover9 left a comment

Choose a reason for hiding this comment

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

Code looks absolutely fantastic! Will do some testing shortly but I don't see any issues - thank you for working so much on this!

@gdover9
Copy link

gdover9 commented Mar 13, 2025

All looks good in terms of *.json, *.csv and *_test.go files not including sensitive information!

@gdover9
Copy link

gdover9 commented Mar 13, 2025

Have tested deployments --commits and deployed-commits commands on multiple repos and environments and all seems to be correct in terms of data returned :)

@hackebrot hackebrot requested a review from dlactin March 17, 2025 10:50
@hackebrot
Copy link
Contributor Author

Thanks for the reviews, folks! 📦 🚀

@hackebrot hackebrot added this pull request to the merge queue Mar 20, 2025
Merged via the queue into main with commit 89699fa Mar 20, 2025
3 checks passed
@hackebrot hackebrot deleted the add-capability-to-query-deployed-commits branch March 20, 2025 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app:metrics Changes related to the metrics CLI app feature New feature or request pkg:github Changes related to pkg/github

Projects

None yet

4 participants