Skip to content

Codeserver#18

Closed
ericbusboom wants to merge 4 commits intomasterfrom
codeserver
Closed

Codeserver#18
ericbusboom wants to merge 4 commits intomasterfrom
codeserver

Conversation

@ericbusboom
Copy link
Contributor

No description provided.

@ericbusboom ericbusboom requested a review from Copilot May 8, 2025 03:26
Copy link

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 introduces several changes to support the Codeserver setup for the project, including bug fixes in lesson scripts, updates to container configuration files, and new scripts for development setup.

  • Fixed typo in lesson comments in 02_Meet_Tina.py
  • Updated port mappings and container configurations in docker-compose.yaml, devcontainer.json, and Dockerfile
  • Added new devcontainer scripts and a README for container usage

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lessons/00_Turtles/02_Meet_Tina.py Corrected typo in comment
lessons/00_Turtles/01_Get_Started.ipynb Minor documentation change regarding Codespaces naming
docker-compose.yaml Added new service configuration with port mappings
.devcontainer/scrape-display.sh Added a script to capture display frames
.devcontainer/jtl-setup.sh New setup script that moves lesson files and performs repository setup
.devcontainer/install-extensions.sh Script to install VS Code extensions
.devcontainer/devcontainer.json Updated container configuration with new ports and run arguments
.devcontainer/README.md Added documentation for running the dev container
.devcontainer/Dockerfile Switched base image and added code-server installation commands
Comments suppressed due to low confidence (1)

lessons/00_Turtles/01_Get_Started.ipynb:11

  • Please update 'Github' to 'GitHub' to maintain consistency with standard naming conventions used elsewhere in the project.
"website, it should be Github Codespaces."

container_name: PythonApprentice
ports:
- 6080
- 8082:8082
Copy link

Copilot AI May 8, 2025

Choose a reason for hiding this comment

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

The devcontainer service maps port 8082, but other configurations (devcontainer.json and Dockerfile) use port 8080 for code-server. Consider aligning the port mapping to avoid connectivity issues.

Suggested change
- 8082:8082
- 8080:8080

Copilot uses AI. Check for mistakes.
Comment on lines +5 to +6
#cd "$TARGET_DIR" || exit 1

Copy link

Copilot AI May 8, 2025

Choose a reason for hiding this comment

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

[nitpick] The command aggressively removes several directories and files; ensure this removal is fully intentional and consider adding safeguards to prevent accidental data loss.

Suggested change
#cd "$TARGET_DIR" || exit 1
# Validate TARGET_DIR
if [ -z "$TARGET_DIR" ] || [ ! -d "$TARGET_DIR" ]; then
echo "Error: TARGET_DIR is not set or does not exist."
exit 1
fi
# Confirm deletion
echo "The following files and directories will be removed:"
echo ".devcontainer .github .lib requirements.txt LICENSE"
read -p "Are you sure you want to proceed? (y/N): " confirm
if [ "$confirm" != "y" ]; then
echo "Aborting cleanup."
exit 0
fi
# Perform cleanup

Copilot uses AI. Check for mistakes.
@ericbusboom ericbusboom closed this May 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments