Skip to content

Conversation

@jovnc
Copy link
Collaborator

@jovnc jovnc commented Oct 28, 2025

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
Copy link
Collaborator Author

jovnc commented Oct 28, 2025

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.

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.

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 woojiahao added exercise review Review a proposed exercise discussing labels Nov 10, 2025
@jovnc
Copy link
Collaborator Author

jovnc commented Nov 10, 2025

@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:

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

@jovnc jovnc requested a review from woojiahao November 10, 2025 12:50
@VikramGoyal23
Copy link
Collaborator

LGTM, let's wait for @woojiahao's input on the change to the tag function.

@jovnc
Copy link
Collaborator Author

jovnc commented Dec 12, 2025

Given there is a new utility function tag_with_options, I'll refactor to use that instead

@jovnc jovnc force-pushed the feature/tags-update branch from e9d1f3a to c32e219 Compare December 12, 2025 09:40
@jovnc jovnc force-pushed the feature/tags-update branch from c32e219 to 8903401 Compare December 12, 2025 09:45
@jovnc jovnc requested a review from VikramGoyal23 December 12, 2025 09:49
@jovnc
Copy link
Collaborator Author

jovnc commented Dec 12, 2025

@VikramGoyal23 @woojiahao I have made some changes, and would like to ask for a review again, thanks! If there are any changes, do let me know and I'll quickly make the changes, otherwise we can merge this PR

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

Labels

discussing exercise review Review a proposed exercise

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Exercise Discussion] T4L2/tags-update

3 participants