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

ref: CommitContextClient methods accept repository instance #87312

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jianyuan
Copy link
Contributor

@jianyuan jianyuan commented Mar 18, 2025

The issue is that repo.name works well for GitHub (it’s in the format getsentry/sentry, which can be used in the API URL path), whereas repo.name in GitLab is name_with_namespace (formatted as Get Sentry / Sentry), which is not suitable for use in the URL path.

GitLab, in particular, offers an option to use the internal project_id in URL paths.
https://docs.gitlab.com/api/commits/#list-merge-requests-associated-with-a-commit

I propose a change to make the get_merge_commit_sha_from_commit accept a more generic instance of Repository, leaving the implementation details to the integration. I've also adjusted CommitContextIntegration.queue_comment_task_if_needed which uses this method accordingly.

@jianyuan jianyuan requested review from a team as code owners March 18, 2025 19:14
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 18, 2025
@jianyuan jianyuan marked this pull request as draft March 18, 2025 19:25
@jianyuan jianyuan force-pushed the ref/commit-context-client-methods-accept-repository-instance branch from d750154 to 8bd62c7 Compare March 18, 2025 19:36
@jianyuan jianyuan marked this pull request as ready for review March 18, 2025 19:36
@iamrajjoshi iamrajjoshi added the Trigger: getsentry tests Once code is reviewed: apply label to PR to trigger getsentry tests label Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components Trigger: getsentry tests Once code is reviewed: apply label to PR to trigger getsentry tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants