-
Notifications
You must be signed in to change notification settings - Fork 26
Implement exercise T6L2/branch-forward #118
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?
Implement exercise T6L2/branch-forward #118
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.
Left a preliminary question about the autograding schema! I'll review the unit tests when we flesh out the autograding
jovnc
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 comments on verification process.
| @@ -0,0 +1,26 @@ | |||
| # branch-forward | |||
|
|
|||
| You are outlining a story and experimenting with different plotlines. Each version lives on its own branch, and you now need to fold the right storyline back into `main` without cluttering the history. | |||
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.
| raise exercise.wrong_answer([ONLY_WITH_SALLY_MERGED]) | ||
|
|
||
| return exercise.to_output( | ||
| ["Great job fast-forward merging only 'with-sally' and cleaning up the branch!"], |
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.
Should we remove 'cleaning up the branch', as it seems to be redundant and can also add to confusion (can be implied as expecting with-sally branch to be deleted)?
|
|
||
| def verify(exercise: GitAutograderExercise) -> GitAutograderOutput: | ||
| main_branch = exercise.repo.branches.branch("main") | ||
| merge_logs = [ |
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 feel like using reflog for verification might be too restrictive here.
Consider the following scenario: User merges wrong branch, then undoes it, then correctly merges with-sally
git merge with-ginny --ff # Wrong merge
git reset --hard HEAD^ # Undo the merge
git merge with-sally --ff # Correct merge
The reflog retains history even after reset.
This means that even after the student correctly recovers from his/her mistake, he will still likely fail the test at line 36.
if any(branch != "with-sally" for branch in merged_branches):
raise exercise.wrong_answer([ONLY_WITH_SALLY_MERGED])|
Hi @keerthigkaarthik , is this PR currently being worked on? Would be good to get an update by 9 Dec, else we need to close this PR, so that we can resolve this issue. Thanks! |
|
Yep, sorry but I'll only be able to get around to it on Tuesday. I'll make
the changes within the day. Thanks!
…On Sat, 6 Dec 2025, 08:52 jovnc, ***@***.***> wrote:
*jovnc* left a comment (git-mastery/exercises#118)
<#118 (comment)>
Hi @keerthigkaarthik <https://github.com/keerthigkaarthik> , is this PR
currently being worked on?
Would be good to get an update by 9 Dec, else we need to close this PR, so
that we can resolve this issue. Thanks!
—
Reply to this email directly, view it on GitHub
<#118 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A4UK3LASUU5ZX66X7THXVZL4AKDMHAVCNFSM6AAAAACMDAHVCOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTMMJZG4YDINBUG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|

Exercise Review
Exercise Discussion
#66
Checklist
Git-Masteryorganization, have you created a request for it?repo-smithto validate the exercise grading scheme?test-download.sh?git-autograder?app?