Skip to content

Conversation

@MousseDm
Copy link
Contributor

@MousseDm MousseDm commented Oct 27, 2025

Exercise Review

Exercise Discussion

#61

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
Copy link
Contributor

damithc commented Oct 28, 2025

@MousseDm Closing this as you already have one hp-* PR

@damithc damithc closed this Oct 28, 2025
@VikramGoyal23
Copy link
Contributor

@damithc This seems to be their exercise implementation, not their hands-on implementation.

@MousseDm You have accidentally left in your implementation of hp-ignore-file in your tags-add branch, in the form of the commit 3f000ca.

@damithc
Copy link
Contributor

damithc commented Oct 28, 2025

@damithc This seems to be their exercise implementation, not their hands-on implementation.

Ah, right. Thanks @VikramGoyal23
reopening.

@damithc damithc reopened this Oct 28, 2025
@damithc damithc linked an issue Oct 28, 2025 that may be closed by this pull request
1 task
@woojiahao woojiahao added the exercise review Review a proposed exercise label Nov 10, 2025
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.

Left several comments. The main points are:

  1. Create unit tests for the autograding to ensure that it works. If there are commands that are insufficient from repo-smith, ping me and I will add them
  2. All comments should be a constant for ease of unit testing
  3. All comments should avoid using the ` character. Use " instead

Comment on lines +1 to +2
__resources__ = []

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
__resources__ = []

Comment on lines +3 to +4
1. Add a **lightweight** tag `first-pilot` to the **first commit**.
2. Add an **annotated** tag `v1.0` to the commit that updates March duty roster, with message `first full duty roster`.
Copy link
Member

Choose a reason for hiding this comment

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

These should go under Task

Comment on lines +12 to +23
```bash
# first commit
git rev-list --max-parents=0 HEAD

# add lightweight tag
git tag first-pilot <FIRST_COMMIT_SHA>

# find the commit for "Update roster for March"
git log --oneline | grep -i "Update roster for March"

# add annotated tag with message
git tag -a v1.0 -m "first full duty roster" <MARCH_COMMIT_SHA> No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Please use the Github collapsible sections.

Comment on lines +12 to +23
```bash
# first commit
git rev-list --max-parents=0 HEAD

# add lightweight tag
git tag first-pilot <FIRST_COMMIT_SHA>

# find the commit for "Update roster for March"
git log --oneline | grep -i "Update roster for March"

# add annotated tag with message
git tag -a v1.0 -m "first full duty roster" <MARCH_COMMIT_SHA> No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

There's a missing closing tag for the code block. Add ``` to the end

Copy link
Member

Choose a reason for hiding this comment

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

We will need unit tests to ensure that the autograding works. Please refer to the other exercises to understand how autograding unit testing works.

comments.append(f"Missing annotated tag `{SECOND_TAG}`.")
else:
if not isinstance(getattr(t_v1, "tag", None), TagObject):
comments.append(f"`{SECOND_TAG}` must be an annotated tag.")
Copy link
Member

Choose a reason for hiding this comment

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

This should be a constant

else:
msg = (t_v1.tag.message or "").strip()
if msg != SECOND_TAG_MSG:
comments.append(f"`{SECOND_TAG}` message must be exactly `{SECOND_TAG_MSG}`.")
Copy link
Member

Choose a reason for hiding this comment

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

This should be a constant

if msg != SECOND_TAG_MSG:
comments.append(f"`{SECOND_TAG}` message must be exactly `{SECOND_TAG_MSG}`.")
if not march_commit:
comments.append(f"Could not find the commit containing '{MARCH_MSG_FRAGMENT}'.")
Copy link
Member

Choose a reason for hiding this comment

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

This should be a constant

Comment on lines +48 to +51
comments.append(
f"`{SECOND_TAG}` should point to March commit {march_commit.hexsha[:7]}, "
f"but points to {t_v1.commit.hexsha[:7]} instead."
)
Copy link
Member

Choose a reason for hiding this comment

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

This should be a constant

Comment on lines +53 to +54
status = GitAutograderStatus.SUCCESSFUL if not comments else GitAutograderStatus.FAILED
return exercise.to_output(comments, status)
Copy link
Member

Choose a reason for hiding this comment

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

We would like to provide positive feedback if they do succeed! So I would opt to provide a positive note for students like "Great work using git tag to annotate various commits in the repository!"

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-add

4 participants