Copilot commented on code in PR #60194:
URL: https://github.com/apache/airflow/pull/60194#discussion_r2666614907
##########
.github/workflows/basic-tests.yml:
##########
@@ -219,21 +219,40 @@ jobs:
python-version: ${{ steps.breeze.outputs.host-python-version }}
platform: ${{ inputs.platform }}
save-cache: true
- - name: Fetch incoming commit ${{ github.sha }} with its parent
+ - name: Fetch incoming commit ${{ github.sha }} with history
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 #
v4.2.2
with:
ref: ${{ github.sha }}
- fetch-depth: 2
+ fetch-depth: 50
persist-credentials: false
+ - name: Calculate merge-base for PR
+ id: merge-base
+ run: |
+ git fetch origin "$BASE_BRANCH" --depth=50
Review Comment:
The hardcoded depth value of 50 is repeated multiple times (lines 226, 231,
238, 239). Consider extracting this as an environment variable or input
parameter to improve maintainability and make it easier to adjust if needed.
##########
.github/workflows/basic-tests.yml:
##########
@@ -219,21 +219,40 @@ jobs:
python-version: ${{ steps.breeze.outputs.host-python-version }}
platform: ${{ inputs.platform }}
save-cache: true
- - name: Fetch incoming commit ${{ github.sha }} with its parent
+ - name: Fetch incoming commit ${{ github.sha }} with history
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 #
v4.2.2
with:
ref: ${{ github.sha }}
- fetch-depth: 2
+ fetch-depth: 50
persist-credentials: false
+ - name: Calculate merge-base for PR
+ id: merge-base
+ run: |
+ git fetch origin "$BASE_BRANCH" --depth=50
+
+ # Try to find merge-base, deepen if not found
+ MERGE_BASE=$(git merge-base "origin/$BASE_BRANCH" HEAD 2>/dev/null
|| echo "")
+
+ if [ -z "$MERGE_BASE" ]; then
+ echo "Merge-base not found with depth=50, fetching more history..."
Review Comment:
The magic number 450 for the fallback deepen operation lacks explanation.
Consider adding a comment explaining why this specific depth was chosen (e.g.,
'Deepen to accommodate PRs with many commits or long-running branches').
```suggestion
echo "Merge-base not found with depth=50, fetching more
history..."
# Deepen history significantly (by an additional 450 commits) to
accommodate PRs with many commits
# or long-running branches where the merge-base is far back in
history.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
