Skip to content

Conversation

@gianbelinche
Copy link
Owner

Motivation

Description

Closes #issue_number

github-merge-queue bot pushed a commit to lambdaclass/ethrex that referenced this pull request Oct 31, 2025
**Motivation**
The L2 CI is not running on some PRs it should.
<!-- Why does this pull request exist? What are its goals? -->

**Description**
The problem is that the required checks `Integration Test` and `Lint`
have the same name for both L1 and L2. So if one of them passes, then
the other is not needed to merge. This can be a problem in the case that
the L1 CI finishes before the L2 one starts, in that case the L2 CI will
not run.

The solution is the following:
- Add `Integration Test L2` and `Lint L2` to the branch protection rules
- Rename the corresponding jobs on the L2 workflow.

With only those changes, we will always require the L2 workflows to run
in order to merge, since we want to be able only run the L1 CI in PRs
that don't touch the L2, we need a way to skip it.

Note: The current way of not running a workflow doesn't work, since
skipping a complete workflow with `paths:` makes the jobs abscent, what
we need is for the jobs to be marked as skipped.

To do this,
[dorny/paths-filter@v3](https://github.com/dorny/paths-filter) action is
used, which performs the same action as the `paths:` option, but as a
job itself.
Then with the result of this job we can add ifs to the rest of the jobs
to mark if they should be run.

Examples with a fork of ethrex with the branch protection already active
(For Integration Test L2)
- [PR with changes only in
L1](gianbelinche#4): It ran L1 checks and
skipped L2 ones.
- [PR with changes only in
L2](gianbelinche#3): It ran L2 checks and
skipped L1 ones.
- [PR with both L1 and L2
changes](gianbelinche#5): It ran both L1
and L2 checks.

You can also check that when the PRs were merged to main, all checks ran
even though the PRs skipped the checks.
https://github.com/gianbelinche/ethrex/commits/main/

<!-- A clear and concise general description of the changes this PR
introduces -->

<!-- Link to issues: Resolves #111, Resolves #222 -->
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.

2 participants