Implement exercise T4L2/tags-update #91

jovnc · 2025-10-28T13:10:40Z

Exercise Review

Exercise Discussion

Fixes #62

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?

jovnc · 2025-10-28T13:31:44Z

tags_update/verify.py

+MISSING_COMMIT_MESSAGE = "Could not find a commit with '{message}' in the message"
+
+
+def find_commit_by_message(exercise: GitAutograderExercise, message: str):


Similar in functionality with get_target_commit_sha from log_and_order/verify.py, but more flexible and simpler logic.

jovnc · 2025-10-28T13:34:46Z

Opened PR early while waiting to be assigned. Feel free to close if there are duplicates in the future, or if someone else is assigned to this exercise in the future.

This would serve as potential reference to others looking for implementations of exercises as well.

woojiahao

Thank you @jovnc for working on this exercise! I think that there are some major comments on the ordering of grading, less on the coding style, more so on the autograding philosophy. I will need to update the public documentation to capture this logic too

woojiahao · 2025-11-10T03:52:22Z

tags_update/download.py

+    run_command(["git", "tag", "first-update", "HEAD~4"], verbose)
+    run_command(["git", "tag", "april-update"], verbose)
+    run_command(["git", "tag", "may-update"], verbose)


Given that this is repeated multiple times, could we get it as a utility function for future use?

tags_update/download.py

tags_update/verify.py

woojiahao · 2025-11-10T03:59:09Z

tags_update/verify.py

+    # Verify that the tags exist and that the old first-update tag is deleted
+    if "first-update" in tags:
+        raise exercise.wrong_answer([OLD_FIRST_UPDATE_TAG])    
+    if "january-update" not in tags:
+        raise exercise.wrong_answer([MISSING_JANUARY_TAG])
+    if "april-update" not in tags:
+        raise exercise.wrong_answer([MISSING_APRIL_TAG])
+
+    # Get correct commits that the tags should point to
+    january_commit = find_commit_by_message(exercise, "Add January duty roster")
+    if january_commit is None:
+        raise exercise.wrong_answer([MISSING_COMMIT_MESSAGE.format(message="January")])
+
+    april_commit = find_commit_by_message(exercise, "Update duty roster for April")
+    if april_commit is None:
+        raise exercise.wrong_answer([MISSING_COMMIT_MESSAGE.format(message="April")])
+
+    # Verify that the tags point to the correct commits
+    january_tag_commit = tags["january-update"].commit
+    if january_tag_commit.hexsha != january_commit.hexsha:
+        raise exercise.wrong_answer([WRONG_JANUARY_TAG_COMMIT])
+
+    april_tag_commit = tags["april-update"].commit
+    if april_tag_commit.hexsha != april_commit.hexsha:
+        raise exercise.wrong_answer([WRONG_APRIL_TAG_COMMIT])
+
+    return exercise.to_output(
+        [SUCCESS_MESSAGE],
+        GitAutograderStatus.SUCCESSFUL,
+    )


I think it would make more sense that the grading following the order of the tasks provided. The rationale is that students may attempt each task and run gitmastery verify immediately after. So if the autograding provides feedback for tasks later down the line, they might not really understand the feedback.

For instance, in this exercise, the first task is to rename the first-update tag to january-update. So you would want to

Ensure that first-update does not exist

Ensure that january-update exists

Ensure that the january-update tag points to the right commit

Other than that, I like the comprehensiveness of the checks. But I think it would greatly benefit from a little reshuffling!

woojiahao · 2025-11-10T04:01:45Z

tags_update/tests/test_verify.py

+def test_wrong_april_tag():
+    with loader.load("specs/wrong_april_tag.yml", "start") as output:
+        assert_output(output, GitAutograderStatus.UNSUCCESSFUL, [WRONG_APRIL_TAG_COMMIT])


I think we would also want to ensure that the same check for the January tag commit should exist too, what do you think?

That's a good point, would make the tests more comprehensive!

tags_update/tests/test_verify.py

jovnc · 2025-11-10T12:34:59Z

exercise_utils/git.py

-def tag(tag_name: str, verbose: bool) -> None:
-    """Tags the latest commit with the given tag_name."""
-    run_command(["git", "tag", tag_name], verbose)
+def tag(tag_name: str, verbose: bool, commit_ref: str | None = None) -> None:


Given that this is repeated multiple times, could we get it as a utility function for future use?

There is an existing tag function under exercise_utils.git, however, it doesn't support tagging a specific commit hash.

We have 2 options:

extend current utility of the tag function to take in optional commit_ref

create a command with a more verbose name tag_with_commit that serves the same purpose as tag but takes in compulsory commit_ref

I have chosen the first option, as it keeps backward compatibility, while ensuring simplicity.

Do I need to open a new PR to add this change for the util, or can I include it in this PR? Is there anywhere I need to document this change?

jovnc · 2025-11-10T12:50:23Z

@woojiahao Thanks for the PR review, and the very detailed comments. I have made the changes based on your comments, namely here are the main changes:

modified tag util function to also take in optional parameter commit_ref
add new test for wrong january-tag
reorder verification process to follow order of the task

jovnc added 4 commits October 28, 2025 21:07

[tags-update] Implement download and verify

0fda567

[tags-update] Add tests

4942bf7

[tags-update] Add ending line

d1f48db

[tags-update] Add more strict verification

c7998e6

jovnc commented Oct 28, 2025

View reviewed changes

woojiahao requested changes Nov 10, 2025

View reviewed changes

woojiahao added exercise review Review a proposed exercise discussing labels Nov 10, 2025

[tags-update] Enhance tagging functionality

82b50ca

jovnc commented Nov 10, 2025

View reviewed changes

[tags-update] Reorder verification process and add more tests

c53cbe5

jovnc requested a review from woojiahao November 10, 2025 12:50

		MISSING_COMMIT_MESSAGE = "Could not find a commit with '{message}' in the message"


		def find_commit_by_message(exercise: GitAutograderExercise, message: str):

Implement exercise T4L2/tags-update #91

Are you sure you want to change the base?

Implement exercise T4L2/tags-update #91

Uh oh!

Conversation

jovnc commented Oct 28, 2025

Exercise Review

Exercise Discussion

Checklist

Uh oh!

jovnc Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

jovnc commented Oct 28, 2025

Uh oh!

woojiahao left a comment

Choose a reason for hiding this comment

Uh oh!

woojiahao Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

woojiahao Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

woojiahao Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

woojiahao Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

jovnc Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jovnc Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jovnc Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

jovnc commented Nov 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jovnc Nov 10, 2025 •

edited

Loading