Copilot commented on code in PR #149:
URL: https://github.com/apache/datafusion-site/pull/149#discussion_r2779092067
##########
Makefile:
##########
@@ -32,15 +31,11 @@ clone-repo:
echo "$(REPO_NAME) already exists, skipping clone."; \
fi
-# checks out the specific commit due to
https://github.com/apache/infrastructure-actions/issues/218
+# pulls the latest changes from the main branch
checkout-commit: clone-repo
@cd $(REPO_NAME) && \
Review Comment:
The target name `checkout-commit` no longer checks out a commit hash; it now
tracks `main`. Renaming this target (and its dependency in `build`) to
something like `update-repo`/`sync-main` would make the Makefile behavior
clearer and reduce future confusion.
##########
Makefile:
##########
@@ -32,15 +31,11 @@ clone-repo:
echo "$(REPO_NAME) already exists, skipping clone."; \
fi
-# checks out the specific commit due to
https://github.com/apache/infrastructure-actions/issues/218
+# pulls the latest changes from the main branch
checkout-commit: clone-repo
@cd $(REPO_NAME) && \
- if [ "$$(git rev-parse HEAD)" = "$(COMMIT_HASH)" ]; then \
- echo "Repository is already at commit $(COMMIT_HASH), skipping
checkout."; \
- else \
- echo "Checking out commit $(COMMIT_HASH)..."; \
- git fetch --depth 1 origin $(COMMIT_HASH) && git checkout
$(COMMIT_HASH); \
- fi
+ echo "Pulling latest changes from main branch..."; \
+ git fetch origin main && git checkout main && git pull origin main
Review Comment:
`clone-repo` uses a shallow clone (`--depth 1`), but the new `git fetch
origin main` is not depth-limited and can negate the shallow clone by
downloading more history over time. Consider keeping fetch shallow (e.g., fetch
with `--depth 1` or otherwise ensure the checkout stays shallow) to keep this
target fast and consistent.
```suggestion
git fetch --depth 1 origin main && git checkout main && git pull
--depth 1 origin main
```
##########
Makefile:
##########
@@ -32,15 +31,11 @@ clone-repo:
echo "$(REPO_NAME) already exists, skipping clone."; \
fi
-# checks out the specific commit due to
https://github.com/apache/infrastructure-actions/issues/218
+# pulls the latest changes from the main branch
checkout-commit: clone-repo
@cd $(REPO_NAME) && \
- if [ "$$(git rev-parse HEAD)" = "$(COMMIT_HASH)" ]; then \
- echo "Repository is already at commit $(COMMIT_HASH), skipping
checkout."; \
- else \
- echo "Checking out commit $(COMMIT_HASH)..."; \
- git fetch --depth 1 origin $(COMMIT_HASH) && git checkout
$(COMMIT_HASH); \
- fi
+ echo "Pulling latest changes from main branch..."; \
+ git fetch origin main && git checkout main && git pull origin main
Review Comment:
The `git fetch ... && git checkout main && git pull origin main` sequence
can produce non-deterministic results (e.g., merge commits or failures if the
local `main` has diverged). For a build script, prefer a deterministic sync to
`origin/main` (e.g., explicitly set the local branch to track `origin/main` and
fast-forward/reset to it) and consider failing early if the repo has
uncommitted changes.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
