-
Notifications
You must be signed in to change notification settings - Fork 217
CI: Add script and automated workflow to clean up stale PR preview documentation folders #1031
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: main
Are you sure you want to change the base?
Conversation
…ocumentation Co-authored-by: leofang <[email protected]>
Co-authored-by: leofang <[email protected]>
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.
@copilot help me reduce my review burden, see my comments below.
Co-authored-by: leofang <[email protected]>
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.
Co-authored-by: leofang <[email protected]>
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.
Co-authored-by: leofang <[email protected]>
|
pre-commit.ci autofix |
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.
bab10c6 to
241923e
Compare
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.
@copilot I force-pushed to revert a commit you made on fixing the 404 error. It was incorrect. I then added another commit to fix temp dir & ensure the latest gh-pages branch is checked out locally. Please review my changes and address my comment.
| # Cleanup function to properly remove worktree and temp directory | ||
| cleanup_worktree() { | ||
| cd - >/dev/null 2>&1 || true # Go back to original directory | ||
| if [[ -n "$TEMP_DIR" && -d "$TEMP_DIR" ]]; then | ||
| git worktree remove "$TEMP_DIR" --force >/dev/null 2>&1 || true | ||
| fi | ||
| rm -rf "$TEMP_DIR" >/dev/null 2>&1 || true | ||
| } | ||
| trap cleanup_worktree EXIT |
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.
This cleanup should not be executed if the changes made to the local worktree is not yet pushed, otherwise all local changes are lost after the script ends.
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.
Fixed in commit 2a7b4cc. The cleanup function now checks if changes have been pushed before removing the worktree. When --push is not used, the worktree is preserved to prevent loss of unpushed changes, and the script provides instructions for manual cleanup. The cleanup only occurs automatically when changes are successfully pushed or when no changes were made.
Co-authored-by: leofang <[email protected]>
ci/cleanup-pr-previews
Outdated
| # Create a git worktree for gh-pages branch | ||
| TEMP_DIR="./gh-pages-cleanup" | ||
| rm -rf ${TEMP_DIR} | ||
| mkdir ${TEMP_DIR} |
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.
@copilot this change I made is not fully safe. If locally we already have a worktree at $TEMP_DIR, rm -rf it does not also remove the worktree. Could you figure out a safer solution?
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.
Fixed in commit 49bdbce. The script now properly handles existing worktrees by using git worktree remove before rm -rf to clean up git registrations. Added retry logic that detects leftover worktree registrations and attempts cleanup if the initial worktree creation fails. This prevents issues when re-running the script with the same temp directory.
Co-authored-by: leofang <[email protected]>
|
What's the status of this PR? It seems like it's been languishing for a while. Let's make a decision and either merge it or close it out. |
|
@copilot Please add a CI workflow that runs this script every night at 11pm EST. Also make it manually dispatchable. |
Co-authored-by: leofang <[email protected]>
Added in commit f4d0dfa. Created
|
|
pre-commit.ci autofix |
git worktree removebeforerm -rfto clean up git registrations.github/workflows/cleanup-pr-previews.ymlScript Interface
The cleanup script can be run manually with the following options:
Automated Workflow
The workflow runs automatically every night at 11pm EST and can also be manually triggered from the GitHub Actions tab:
The script safely handles existing worktrees and git registrations, preventing issues when re-running with the same temp directory name.
Fixes #628
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.