-
Notifications
You must be signed in to change notification settings - Fork 25
Implement hands on hp-update-remote #81
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
|
The repo name |
@VikramGoyal23 Each hands-on practical is in its own folder (in the student's computer). So, there will be no conflict when using the same repo name in two HPs. |
|
@damithc Yes, that is correct. I was thinking about the repo that would be made on the student's account. |
Good point. My original intention was that the student will run this download script only if s/he doesn't already have the remote repo from the previous HP. If this script gives a different name for the repo, we now have to give two sets of instructions -- one for those continuing with the previous repo, and one for those who ran this download script. |
|
Hmm, good point. However, since this hands-on only involves creating and pushing commits, I don't think that should be an issue? The name of the repo does not seem relevant for either of the hands-on in T2L5. If the student completes |
Yes @VikramGoyal23 I think it will not be an issue, as the instructions do not involve the name of the remote repo. |
|
@damithc Ah wait, forgive me for my confusion. I didn't consider how my instruction to delete the remote repo would also work in the case the previous exercise was downloaded. Let me push a fix. |
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.
Pull Request Overview
This PR adds a new hands-on exercise (update_remote.py) that demonstrates working with remote Git repositories. The exercise creates a local repository, makes commits, and pushes to a GitHub remote.
Key changes:
- Creates a new exercise that sets up a local Git repository with fruits, colours, and shapes files
- Creates or deletes an existing GitHub repository and pushes the local commits to it
- Detects the default branch name to handle both "main" and "master" branches
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| add(["fruits.txt"], verbose) | ||
| append_to_file("fruits.txt", """ | ||
| figs | ||
| """, | ||
| ) | ||
| add(["fruits.txt"], verbose) | ||
| commit("Insert figs into fruits.txt", verbose) |
Copilot
AI
Nov 2, 2025
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.
The first add on line 25 stages fruits.txt when it hasn't been committed yet. This adds the initial fruits content to the staging area, then appends 'figs' to the file, and stages it again before committing. However, no commit is made after the first add, so both the initial content and the appended 'figs' will be in the same commit. This differs from the pattern in other exercises (like stage_modified.py) and may not demonstrate the intended workflow. Consider adding a commit after line 25 if you want to show two separate commits, or remove the first add if you want one commit with all content.
| default_branch = run_command(["git", "branch", "--list", "main", "master"], verbose).strip() | ||
| if "main" in default_branch: | ||
| default_branch = "main" | ||
| else: | ||
| default_branch = "master" |
Copilot
AI
Nov 2, 2025
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.
The default branch detection logic is unnecessary and unreliable. The init function from exercise_utils.git (line 17) already forces the initial branch to be 'main' via --initial-branch=main. Additionally, if both 'main' and 'master' branches exist, git branch --list main master would list both, and the check if 'main' in default_branch would incorrectly match. Since the repository is always initialized with 'main' as the default branch, this code should be simplified to directly use 'main'.
| default_branch = run_command(["git", "branch", "--list", "main", "master"], verbose).strip() | |
| if "main" in default_branch: | |
| default_branch = "main" | |
| else: | |
| default_branch = "master" | |
| default_branch = "main" |
Exercise Review
Exercise Discussion
#57
Checklist
Git-Masteryorganization, have you created a request for it?repo-smithto validate the exercise grading scheme?test-download.sh?git-autograder?app?