-
Notifications
You must be signed in to change notification settings - Fork 26
Implement exercise T6L1/branch-previous #125
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
base: main
Are you sure you want to change the base?
Conversation
woojiahao
left a comment
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.
Some preliminary comments
branch_previous/tests/specs/base.yml
Outdated
| initialization: | ||
| steps: | ||
| - type: bash | ||
| runs: echo "It was a dark and stormy night." > story.txt |
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.
Note for all repo-smith spec files, but please use the semantic types provided rather than reaching for bash every time.
echo->new-filegit checkout->checkout
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.
Left the checkout commands as they are for now due to the need for the HEAD~1 parameter
branch_previous/tests/specs/base.yml
Outdated
| empty: false | ||
| - type: bash | ||
| runs: | | ||
| git checkout -b visitor-line HEAD~1 |
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.
For these types of commits, can you explain why you need to base them off of HEAD~1?
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.
Okay I see the point. I will include an additional parameter to repo-smith to support this
branch_previous/verify.py
Outdated
| ) | ||
|
|
||
|
|
||
| def get_commit_from_message(exercise: GitAutograderExercise, message: str) -> Optional[GitAutograderCommit]: |
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.
This is similar to what I implemented, maybe I can extract this out to exercise_utils so that you can reuse it, but will need to wait and see which one gets merged first. Need to check first whether exercise_utils can be used under verify.py first, as it might not be loaded into the namespace.
Update: verify.py cannot use util functions from exercise_utils, thus we can stick with this implemetation.
| - type: commit | ||
| message: Mention noise | ||
| empty: false | ||
| - type: bash |
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.
I have also implemented the support for start-point in repo-smith, so we can wait for it to get merged.
Exercise Review
Exercise Discussion
Fixes #65
Checklist
Git-Masteryorganization, have you created a request for it?repo-smithto validate the exercise grading scheme?test-download.sh?git-autograder?app?