Skip to content

ci(l1,l2): lines of code report #3764

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

Merged
merged 18 commits into from
Jul 24, 2025
Merged

ci(l1,l2): lines of code report #3764

merged 18 commits into from
Jul 24, 2025

Conversation

gianbelinche
Copy link
Contributor

@gianbelinche gianbelinche commented Jul 22, 2025

Motivation
The lines of code CI compares directly with main, so if the branch is not updated it gives fake results

Description
Now the CI compares to the most common commit between the branches

How to Test

This PR is not up to date with main, without the changes on the CI we would see a code report with the new changes on main.
Since this PR only changes the CI there should be no code changes on the report, but in order to show that it works, a commit was added where one line changed, a report was generated, and then the commit was reverted.
The old report is still present since the new run of the CI detected that there are no changes, so the comment is not updated.

@gianbelinche gianbelinche changed the title fix(ci): lines of code ci ci(l1,l2): lines of code report Jul 22, 2025
@github-actions github-actions bot added the L2 Rollup client label Jul 22, 2025
@gianbelinche gianbelinche added L1 Ethereum client ci Github actions, build, tests, etc labels Jul 22, 2025
@gianbelinche gianbelinche marked this pull request as ready for review July 22, 2025 16:04
@Copilot Copilot AI review requested due to automatic review settings July 22, 2025 16:04
@gianbelinche gianbelinche requested a review from a team as a code owner July 22, 2025 16:04
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves the accuracy of the lines of code (LOC) comparison in the CI workflow by changing the comparison baseline from the PR base branch to the merge base commit between the PR and base branches. This prevents inaccurate results when the PR branch is not up-to-date with the main branch.

  • Updates the checkout process to first checkout the PR code, then find and checkout the merge base commit
  • Adds a new step to calculate the merge base between head and base branches using git merge-base
  • Replaces direct base branch comparison with merge base comparison for more accurate LOC reporting

run: |
git fetch --depth=100000 origin $HEAD_REF
git fetch --depth=100000 origin $BASE_REF
MERGE_BASE=$(git merge-base origin/$HEAD_REF origin/$BASE_REF)
Copy link
Preview

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The git merge-base command could fail if the branches don't share common history or if the fetch operations didn't retrieve sufficient commits. Add error handling to check if the merge base was found successfully.

Suggested change
MERGE_BASE=$(git merge-base origin/$HEAD_REF origin/$BASE_REF)
MERGE_BASE=$(git merge-base origin/$HEAD_REF origin/$BASE_REF || echo "")
if [ -z "$MERGE_BASE" ]; then
echo "Error: Unable to find a merge base between $HEAD_REF and $BASE_REF." >&2
exit 1
fi

Copilot uses AI. Check for mistakes.

Copy link

Lines of code report

Total lines added: 1
Total lines removed: 0
Total lines changed: 1

Detailed view
+-------------------------------------------------+-------+------+
| File                                            | Lines | Diff |
+-------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/l1_proof_verifier.rs | 270   | +1   |
+-------------------------------------------------+-------+------+

@github-project-automation github-project-automation bot moved this to In Review in ethrex_l1 Jul 24, 2025
@gianbelinche gianbelinche added this pull request to the merge queue Jul 24, 2025
Merged via the queue into main with commit c984865 Jul 24, 2025
44 checks passed
@gianbelinche gianbelinche deleted the loc-fix branch July 24, 2025 14:27
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Jul 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Github actions, build, tests, etc L1 Ethereum client L2 Rollup client
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants