-
Notifications
You must be signed in to change notification settings - Fork 25
Implement exercise T4L2/tags-add #85
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| { | ||
| "exercise_name": "tags_add", | ||
| "tags": ["git-tag"], | ||
| "requires_git": true, | ||
| "requires_github": true, | ||
| "base_files": {}, | ||
| "exercise_repo": { | ||
| "repo_type": "remote", | ||
| "repo_name": "duty-roster", | ||
| "create_fork": false, | ||
| "repo_title": "gm-duty-roster", | ||
| "init": false | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| # tags-add | ||
|
|
||
| 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`. | ||
|
|
||
| ## Task | ||
|
|
||
| This exercise clones `git-mastery/gm-duty-roster` into the `duty-roster/` folder. | ||
|
|
||
| ## Hints | ||
|
|
||
| ```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> | ||
|
Comment on lines
+12
to
+23
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use the Github collapsible sections.
Comment on lines
+12
to
+23
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,4 @@ | ||||
| __resources__ = [] | ||||
|
|
||||
|
Comment on lines
+1
to
+2
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
| def setup(verbose: bool = False): | ||||
| pass | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| initialization: | ||
| steps: | ||
| - type: commit | ||
| empty: true | ||
| message: Empty commit | ||
| id: start |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| from git_autograder import GitAutograderTestLoader | ||
|
|
||
| from ..verify import verify | ||
|
|
||
| REPOSITORY_NAME = "tags-add" | ||
|
|
||
| loader = GitAutograderTestLoader(__file__, REPOSITORY_NAME, verify) | ||
|
|
||
|
|
||
| def test_base(): | ||
| with loader.load("specs/base.yml", "start"): | ||
| pass |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,54 @@ | ||||||
| from git import TagObject | ||||||
| from git_autograder.core import GitAutograderExercise, GitAutograderStatus | ||||||
|
|
||||||
| FIRST_TAG = "first-pilot" | ||||||
| SECOND_TAG = "v1.0" | ||||||
| SECOND_TAG_MSG = "first full duty roster" | ||||||
| MARCH_MSG_FRAGMENT = "Update roster for March" | ||||||
|
|
||||||
| def verify(exercise: GitAutograderExercise): | ||||||
| repo = exercise.repo.repo | ||||||
|
|
||||||
| root_sha = next(repo.iter_commits(rev="HEAD", reverse=True)).hexsha | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A more ideal way to do it is via: exercise.repo.branches.branch("main").commits[-1].hexshaThese are all built-in for git-autograder, so would like to minimize the amount of raw GitPython written! |
||||||
|
|
||||||
| march_commit = None | ||||||
| for c in repo.iter_commits("HEAD"): | ||||||
| if MARCH_MSG_FRAGMENT in c.message: | ||||||
|
Comment on lines
+15
to
+16
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inline with the above, you should iterate over the commits through the branch: for c in exercise.repo.branches.branch("main").commits |
||||||
| march_commit = c | ||||||
| break | ||||||
|
Comment on lines
+14
to
+18
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our general autograding philosophy is to grade the tasks in order. So if you don't need this variable till task 2, I would suggest moving it down to later on. |
||||||
|
|
||||||
| comments = [] | ||||||
|
|
||||||
| t_first = next((t for t in repo.tags if t.name == FIRST_TAG), None) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that you iterate over the tags of the repository multiple times, it'll be worthwhile storing it under a variable
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Separately, the name of the variable can be improved: |
||||||
| if not t_first: | ||||||
| comments.append(f"Missing lightweight tag `{FIRST_TAG}`.") | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be extracted out as a constant
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| else: | ||||||
| if t_first.commit.hexsha != root_sha: | ||||||
| comments.append( | ||||||
| f"`{FIRST_TAG}` should point to first commit {root_sha[:7]}, " | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| f"but points to {t_first.commit.hexsha[:7]} instead." | ||||||
|
Comment on lines
+28
to
+29
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These should be extracted out as constants as per how the other exercises do it. Makes it easier to unit test |
||||||
| ) | ||||||
| if isinstance(getattr(t_first, "tag", None), TagObject): | ||||||
| comments.append(f"`{FIRST_TAG}` must be a lightweight tag (not annotated).") | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a constant
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| t_v1 = next((t for t in repo.tags if t.name == SECOND_TAG), None) | ||||||
| if not t_v1: | ||||||
| comments.append(f"Missing annotated tag `{SECOND_TAG}`.") | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a constant
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| else: | ||||||
| if not isinstance(getattr(t_v1, "tag", None), TagObject): | ||||||
| comments.append(f"`{SECOND_TAG}` must be an annotated tag.") | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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}`.") | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a constant |
||||||
| if not march_commit: | ||||||
| comments.append(f"Could not find the commit containing '{MARCH_MSG_FRAGMENT}'.") | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a constant |
||||||
| else: | ||||||
| if t_v1.commit.hexsha != march_commit.hexsha: | ||||||
| comments.append( | ||||||
| f"`{SECOND_TAG}` should point to March commit {march_commit.hexsha[:7]}, " | ||||||
| f"but points to {t_v1.commit.hexsha[:7]} instead." | ||||||
| ) | ||||||
|
Comment on lines
+48
to
+51
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a constant |
||||||
|
|
||||||
| status = GitAutograderStatus.SUCCESSFUL if not comments else GitAutograderStatus.FAILED | ||||||
| return exercise.to_output(comments, status) | ||||||
|
Comment on lines
+53
to
+54
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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!" |
||||||
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.
These should go under
Task