Skip to content

Group non-breaking dependabot PRs together to reduce review load #18402

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

anoadragon453
Copy link
Member

@anoadragon453 anoadragon453 commented May 6, 2025

While it's great that we have dependabot updating our PRs, the sheer amount of PRs it produces has a few side effects:

  • It causes the overall synapse-core PR review backlog to be bloated, which leads developers to feel that we're getting underwater.
  • It creates an intimidating number of PRs to review, meaning that dependency updates actually happen less frequently.
  • The review process becomes monotonous as most of the time you're waiting for GitHub's UI to load and just pressing buttons.
  • We use less CI minutes.

Thus it seems necessary to try and group the PRs that don't need very careful review together into a single PR. See the comments in the patch for the chosen rules and the justification.

Further reading: https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/optimizing-pr-creation-version-updates#prioritizing-meaningful-updates

Originally suggested by the Server Products Team, and used successfully in the SBG/TI-Messenger repos.

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct
    (run the linters)

@anoadragon453 anoadragon453 marked this pull request as ready for review May 6, 2025 11:15
@anoadragon453 anoadragon453 requested a review from a team as a code owner May 6, 2025 11:15
Copy link
Member Author

@anoadragon453 anoadragon453 May 6, 2025

Choose a reason for hiding this comment

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

@sandhose questioned in an internal meeting whether this would be compatible with what we do for towncrier changelog entries for dependabot PRs in the release script:

def build_dependabot_changelog(repo: Repo, current_version: version.Version) -> str:
"""Summarise dependabot commits between `current_version` and `release_branch`.
Returns an empty string if there have been no such commits; otherwise outputs a
third-level markdown header followed by an unordered list."""
last_release_commit = repo.tag("v" + str(current_version)).commit
rev_spec = f"{last_release_commit.hexsha}.."
commits = list(git.objects.Commit.iter_items(repo, rev_spec))
messages = []
for commit in reversed(commits):
if commit.author.name == "dependabot[bot]":
message: Union[str, bytes] = commit.message
if isinstance(message, bytes):
message = message.decode("utf-8")
messages.append(message.split("\n", maxsplit=1)[0])
if not messages:
print(f"No dependabot commits in range {rev_spec}", file=sys.stderr)
return ""
messages.sort()
def replacer(match: Match[str]) -> str:
desc = match.group(1)
number = match.group(2)
return f"* {desc}. ([\\#{number}](https://github.com/element-hq/synapse/issues/{number}))"
for i, message in enumerate(messages):
messages[i] = re.sub(r"(.*) \(#(\d+)\)$", replacer, message)
messages.insert(0, "### Updates to locked dependencies\n")
# Add an extra blank line to the bottom of the section
messages.append("")
return "\n".join(messages)

@anoadragon453 anoadragon453 removed the request for review from a team May 7, 2025 09:41
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.

1 participant