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

Avoid coverage upload for merge up pull requests #11736

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented Nov 27, 2024

When there are no conflicts between branches, we create pull requests where the head branch is a branch on the origin repository. That branch points to a commit that should already have coverage information provided by the build that happens after merging a regular pull request.

The thing is, coverage information provided by builds that happen before merging a pull request are associated with the commit of the head repository. This means that when merging up 1.2 into 1.3, the build produces coverage information that is the result of a merge between 1.2 and 1.3, and associates it with 1.2, although it is run on with a codebase that is much closer to 1.3 (and is in fact supposed to become 1.3 after the merge).

This means that when we create a merge up PR from 1.2 to anything else, the coverage information is going to be wrong until a PR targeting 1.2 gets merged.

All commits pushed during the wrong interval get such a warning on Codecov:

2024-11-27-22-05-00

I do not think we need coverage about conflictless merge up PRs more than we need accurate numbers, so I propose we disable the upload for those instead of, say, trying to associate them with the temporary merge commit.

When there are no conflicts between branches, we create pull requests
where the head branch is a branch on the origin repository. That branch
points to a commit that should already have coverage information
provided by the build that happens after merging a regular pull request.

The thing is, coverage information provided by builds that happen before
merging a pull request are associated with the commit of the head
repository. This means that when merging up 1.2 into 1.3, the build
produces coverage information that is the result of a merge between 1.2
and 1.3, and associates it with 1.2, although it is run on with a
codebase that is much closer to 1.3 (and is in fact supposed to become
1.3 after the merge).

This means that when we create a merge up PR from 1.2 to anything else,
the coverage information is going to be wrong until a PR targeting 1.2
gets merged.

I do not think we need coverage about conflictless merge up PRs more
than we need accurate numbers, so I propose we disable the upload for
those instead of, say, trying to associate them with the temporary merge
commit.
@greg0ire greg0ire marked this pull request as draft November 27, 2024 20:46
@greg0ire greg0ire marked this pull request as ready for review November 27, 2024 20:46
@greg0ire
Copy link
Member Author

greg0ire commented Nov 27, 2024

Since the last merge up, there has been movement on 2.20.x, this is why the coverage is accurate.
2024-11-27-21-58-08

I wanted to illustrate the issue by creating a merge up PR, but since the changes in that PR do not touch src, there is no codecov upload.

@greg0ire greg0ire added this to the 2.20.1 milestone Nov 28, 2024
@greg0ire greg0ire merged commit 4bda514 into doctrine:2.20.x Nov 28, 2024
69 checks passed
@greg0ire greg0ire deleted the avoid-coverage-upload branch November 28, 2024 06:54
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.

3 participants