Skip to content

ci: log number of merged PRs into main #80

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
Closed
wants to merge 4 commits into from
Closed

ci: log number of merged PRs into main #80

wants to merge 4 commits into from

Conversation

seaona
Copy link

@seaona seaona commented Jun 23, 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

@metamaskbot metamaskbot added the team-qa team-qa label Jun 23, 2025
@seaona seaona self-assigned this Jun 23, 2025
@HowardBraham
Copy link
Contributor

Have you tried running this? I looked for run logs somewhere but didn't find any.

I suspect that GITHUB_REPOSITORY is going to be undefined when you run this.

@seaona
Copy link
Author

seaona commented Jun 26, 2025 •
edited
Loading

Have you tried running this? I looked for run logs somewhere but didn't find any.
I suspect that GITHUB_REPOSITORY is going to be undefined when you run this.

hi @HowardBraham 👋 Thank you for taking a look! So the work will be completed when I create the 2 platform PRs, one for Extension and Mobile, to add the flow there (like here).
Then all the envars should be set (we are using the same as the log-merge-group-failure).

I haven't run it as I'm unsure how can I run this unless I create the PRs on the platforms 🤔 But it's a great point. I'll create the PRs and then see how can I test it and report back 🙏

@HowardBraham
Copy link
Contributor

Yeah you have to create the platform PRs and test them all together. There's no point in approving and merging this alone, it will never work correctly on the first try.

@HowardBraham HowardBraham changed the title chore: log number of daily merged PRs into main ci: log number of daily merged PRs into main Jun 26, 2025
@seaona seaona changed the title ci: log number of daily merged PRs into main ci: log number of merged PRs into main Jul 8, 2025
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 incrementing the success count. This occurs because the current_number_of_successes variable becomes the string "null" if an existing row for the current date in the Google Sheet has an empty or non-numeric value in column C. The jq expression responsible for parsing the sheet data returns "null" in such cases, which bash cannot use in the arithmetic operation $(("$current_number_of_successes" + 1)).

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

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

Lines 46 to 47 in 4db842b

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
Copy link
Author

seaona commented Jul 8, 2025

closing in favour of #83

@seaona seaona closed this Jul 8, 2025
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

Assignees

@seaona seaona

Labels
team-qa team-qa
Projects
None yet
Milestone
No milestone
Development

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