-
Notifications
You must be signed in to change notification settings - Fork 334
Add make sanitize-pr
to github workflow.
#4765
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -38,4 +38,14 @@ jobs: | |||||||||||||||||||||||||||
cat .gitmodules | ||||||||||||||||||||||||||||
make build | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
- name: Clone wire-server and `make sanitize-pr` | ||||||||||||||||||||||||||||
# if this fails, you need to run it on your PR and commit the changes it makes. | ||||||||||||||||||||||||||||
run: | | ||||||||||||||||||||||||||||
CURRENT_REPO_URL="https://github.com/${GITHUB_REPOSITORY}.git" | ||||||||||||||||||||||||||||
BRANCH_NAME=${GITHUB_REF##*/} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
git clone --branch develop "$CURRENT_REPO_URL" wire-server -b "$BRANCH_NAME" | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The git clone command passes a second -b/--branch option after the destination directory, which git interprets as an extra argument and will fail (e.g. "fatal: repository '-b' does not exist"). Use a single --branch before the repo URL, or implement an explicit fallback. Example: git clone --branch "$BRANCH_NAME" "$CURRENT_REPO_URL" wire-server || git clone --branch develop "$CURRENT_REPO_URL" wire-server (with cleanup as needed).
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||||||
cd wire-server | ||||||||||||||||||||||||||||
make sanitize-pr | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
Comment on lines
+41
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This step reclones the same repository that has already been checked out earlier in the job, adding unnecessary network and I/O cost. You can run make sanitize-pr directly in the existing workspace (remove clone/cd) unless isolation is required; if isolation is needed, add a shallow clone (e.g. --depth 1) and fix the branch logic as noted.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||||||
# tasks to update the submodule reference in wire-docs repo are yet to be added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using ${GITHUB_REF##*/} will produce 'merge' for pull_request refs like refs/pull/123/merge, causing the subsequent clone to target a non-existent branch. Prefer GITHUB_HEAD_REF when the event is pull_request, falling back to GITHUB_REF for push events, e.g.: BRANCH_NAME="${GITHUB_HEAD_REF:-${GITHUB_REF#refs/heads/}}".
Copilot uses AI. Check for mistakes.