-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
Process all parent commits in a merge commit, not only the 1st one? #346
Comments
Hi, One of the core requirements of skip-duplicate-actions is that it needs to make skip-decisions rapidly, it should not take more than two or three seconds, regardless of how big the commit-tree is. That being said, if you submit a PR that traces multiple parents, I would definitely consider merging it if the code is not more complex than the rest of the code. |
Theoretically, if we run both parents checks in parallel, that should not slow down things. And there is unlikely more than 2 parents (merges with Merge button press in GitHub UI are common, and they produce exactly 2 parents). |
I agree that it could be implemented efficiently, so the only question remaining is whether a PR could implement it without too much added complexity. |
@fkirc I think there is actually a simpler way, without rechecking all parents, by still checking only one of them. If we order parent commits by their timestamps (the timestamp when they appeared in a branch), and then just check the most recent one, that should be fully enough to replicate the effect. At least, with a very high probability for sure. |
Makes sense, if we always choose the most recent parent-commit, then the backtracking-algorithm would remain basically unchanged |
Hi.
Can the action check not only the 1st parent of a merge commit for changed files, but ALL parents, searching for the one which has no changed files against, and for which the workflow passed?
Here is an illustration:
We use Merge button, and it produces merge commits (like m4). Each merge commit (m4 on the picture) has not one, but 2 parents: one on the main branch itself (m1) and another one on the feature branch (c3). We can see on the picture that the code in c3 will be identical to the code on m4, and since the workflow passes for c3, it can safely be skipped for m4 too.
In fact, the action behaves like this OFTEN, but sometimes, it still doesn't recognize the situation: instead of comparing the code between c3 and m4, it compares it between m1 and m4, which effectively disables skipping.
Here is likely where it is in the code:
skip-duplicate-actions/src/main.ts
Line 314 in 04a1aeb
I see that it only chooses the 1st parent in the list of parents. But for merge commits (like m4 on the picture above), there are 2 parents, and they BOTH are worth checking.
I think that GitHub API returns parents of a merge commit in an unpredictable order, so sometimes it guesses out the correct parent, and sometimes it does not.
The text was updated successfully, but these errors were encountered: