fix(vcs): improve counting of outgoing commits for merge requests #17533
Conversation
Co-authored-by: nijel <[email protected]>
Co-authored-by: nijel <[email protected]>
Co-authored-by: nijel <[email protected]>
Co-authored-by: nijel <[email protected]>
Co-authored-by: nijel <[email protected]>
Co-authored-by: nijel <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
- Add get_fork_branch_name() helper to avoid code duplication - Use super().count_outgoing() for origin check (looks at pull branch) - Update push() to use the helper method - Update test to use helper method for consistency Co-authored-by: nijel <[email protected]>
Co-authored-by: nijel <[email protected]>
- Add check for should_use_fork(branch) before fork checking - Only check fork branch when fork is actually used for that branch - Add test for non-default branch behavior - Add comment explaining why branch parameter is omitted from super() call Co-authored-by: nijel <[email protected]>
There was a problem hiding this comment.
Pull request overview
This pull request improves the counting of outgoing commits in merge request workflows to prevent creating empty pull requests. The fix addresses issue #17226 by checking commits against both the upstream repository (origin) and the fork remote to accurately determine if there are commits that need to be pushed.
Key changes:
- Extracts fork branch naming logic into a reusable method
- Implements enhanced outgoing commit detection that checks both origin and fork remotes
- Adds comprehensive test coverage for the new behavior
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| weblate/vcs/git.py | Adds get_fork_branch_name() method and overrides count_outgoing() to check both origin and fork remotes for pending commits |
| weblate/vcs/tests/test_vcs.py | Adds two new test cases verifying the count_outgoing behavior after merge and with non-default branches |
| docs/changes.rst | Documents the fix in the changelog under bug fixes |
weblate/vcs/git.py
Outdated
| # Omit branch to look at pull branch, not push branch | ||
| origin_outgoing = super().count_outgoing() |
There was a problem hiding this comment.
The branch parameter should be passed to the parent's count_outgoing() method. Currently, line 901 calls super().count_outgoing() without arguments, but the method signature accepts a branch parameter. This could lead to incorrect behavior when checking outgoing commits for a specific branch, as the parent method would use its default behavior instead of checking the requested branch.
| # Omit branch to look at pull branch, not push branch | |
| origin_outgoing = super().count_outgoing() | |
| origin_outgoing = super().count_outgoing(branch) |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (94.33%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Move should_use_fork check to beginning of method - Pass branch parameter to super() when not using fork - Only omit branch parameter when using fork (to check pull branch) - This ensures branch parameter is correctly respected Co-authored-by: nijel <[email protected]>
Co-authored-by: nijel <[email protected]>
- Wrap credentials access in try-except to catch KeyError - This handles cases where credentials are not configured for the repository - Prevents test failures when fork credentials are missing Co-authored-by: nijel <[email protected]>
- Revert commit 35e3c0e that added KeyError handling - Add setUp method to VCSGitLabTest to set repo URL to gitlab.com - This ensures credentials are properly matched during tests - Credentials were already configured via @override_settings decorator Co-authored-by: nijel <[email protected]>
1 participant
Include both missing from upstream and missing from fork to count. This should avoid creating empty pull requests in some situations.
Fixes #17226