Skip to content

Conversation

@PriyadarshiCharvi
Copy link

Exercise Review

Exercise Discussion

#93

Checklist

  • If you require a new remote repository on the Git-Mastery organization, have you created a request for it?
  • Have you written unit tests using repo-smith to validate the exercise grading scheme?
  • Have you tested the download script using test-download.sh?
  • Have you verified that this exercise does not already exist or is not currently in review?
  • Did you introduce a new grading mechanism that should belong to git-autograder?
  • Did you introduce a new dependency that should belong to app?

@damithc damithc linked an issue Nov 6, 2025 that may be closed by this pull request
Copy link
Member

@woojiahao woojiahao left a comment

Choose a reason for hiding this comment

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

This is pending comments from @damithc on the initial hands-on discussion about whether we need to fork the repository. Other minor nits included

Comment on lines +12 to +19
"""
hp-move-tags (T4L2)
- Fork https://github.com/git-mastery/samplerepo-preferences to user account
- Clone the fork locally as 'samplerepo-preferences'
- Create:
* lightweight tag `v1.0` on HEAD
* annotated tag `v0.9` on HEAD~2 with message "First beta release"
"""
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this

# Ensure GitHub CLI is authenticated
gh_user = run_command(["gh", "api", "user", "--jq", ".login"], verbose).strip()
if not gh_user:
raise RuntimeError("GitHub CLI not authenticated. Run `gh auth login` and retry.")
Copy link
Member

Choose a reason for hiding this comment

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

This is already handled when you set __requires_github__ to True


# Fresh sandbox, if re-downloading
if target_dir.exists():
import shutil
Copy link
Member

Choose a reason for hiding this comment

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

I would avoid local imports, there's no issues with importing it at the start of the script

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Hands-On Discussion] T4L2/hp-move-tags (Deleting/moving tags)

2 participants