Skip to content

ci: log number of merged PRs into main #83

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

ci: log number of merged PRs into main #83

wants to merge 4 commits into from

Conversation

seaona
Copy link

@seaona seaona commented Jul 8, 2025 •
edited
Loading

This PR creates a re-usable workflow which logs the number of PRs that get merged to main. The end goal of this is to extend our metrics, to better understand ci health, by getting the success merge rate, using both these data, and the number of PRs that get kicked out from the merge queue.

This PR is based on the work from @itsyoboieltr (see here)

Some notes:

  • It will log the number of PRs merged in a new column (C) in the same spreadhseet --> the reason for using the same spreadsheet is because this data is going to be used together, so it make sense to keep everything in the same spreadsheet. In the future, we might want to log even more metrics like, a split of which was the failing job that kicked out the PR from the merge queue, etc.
  • A subsequent task is to create alert system that automatically sends a slack message, when the success rate is below a threashold -- to be defined<!--
    Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes:
  • What is the current state of things and why does it need to change?
  • What is the solution your changes offer and how does it work?

Are there any issues or other links reviewers should consult to understand this pull request better? For instance:

  • Fixes #12345
  • See: #67890
    -->

@metamaskbot metamaskbot added the team-qa team-qa label Jul 8, 2025
@seaona seaona mentioned this pull request Jul 8, 2025
Closed
cursor[bot]
cursor bot reviewed Jul 8, 2025
View reviewed changes
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: Bash Arithmetic Error with Null Values

The workflow fails with a bash arithmetic error when attempting to increment the success count for an existing date entry. This occurs because the jq expression, which retrieves the current success count from the spreadsheet, returns the string "null" if the corresponding cell in column C is empty or contains non-numeric data. Bash's arithmetic expansion cannot process the string "null", leading to a syntax error. The script should default the count to 0 if it's "null" before performing the increment.

.github/workflows/log-merge-group-success.yml#L42-L47

github-tools/.github/workflows/log-merge-group-success.yml

Lines 42 to 47 in 4db842b

current_date_index=$(echo "$spreadsheet_data" | jq --arg current_date "$current_date" '(.values | map(.[0])) | (index($current_date) | if . == null then null else . + 1 end)')
current_number_of_successes=$(echo "$spreadsheet_data" | jq --arg current_date "$current_date" '(.values[] | select(.[0] == $current_date) | .[2] | tonumber) // null')
if [ "$current_date_index" == "null" ]; then
curl --silent --header "Authorization: Bearer $token" --header "Content-Type: application/json" --request POST --data "{\"values\":[[\"$current_date\", \"\", 1]]}" https://sheets.googleapis.com/v4/spreadsheets/"$SPREADSHEET_ID"/values/"$SHEET_NAME"!A:C:append?valueInputOption=USER_ENTERED
else
curl --silent --header "Authorization: Bearer $token" --header "Content-Type: application/json" --request PUT --data "{\"values\":[[\"$current_date\", \"\", $(("$current_number_of_successes" + 1))]]}" https://sheets.googleapis.com/v4/spreadsheets/"$SPREADSHEET_ID"/values/"$SHEET_NAME"!A"$current_date_index":C"$current_date_index"?valueInputOption=USER_ENTERED

Fix in Cursor • Fix in Web

Was this report helpful? Give feedback by reacting with 👍 or 👎

@seaona seaona mentioned this pull request Jul 8, 2025
Draft
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cursor cursor[bot] cursor[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
team-qa team-qa
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@seaona @metamaskbot