-
Notifications
You must be signed in to change notification settings - Fork 29
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
Checkout testcases #25
Conversation
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.
@rlizzo I would like to hear your thoughts in a couple of cases and hence keeping it incomplete. Would be great if you can review the #TODOs and we can take it from there
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.
In general, these are fairly well thought out tests for the basics of working with checkout and merge commits. A few comments are left below, but nothing major aside from one or two of the (potential) bugs you've found (nice job catching them btw!)
Nice work sherin, much appreciated!
So I'm a bit confused as to the status of this? I think there's a lot of overlap with #32, and this branch doesn't seem like it's really kept up with the changes in master. Do you want to continue with this PR? Or include the relevant changes in #32 along with a most focused commit for the remainder? |
Ok sounds good. I'm just going to rerun the tests to make sure everything is OK, then will merge this and rebase #32. |
Description
Test cases & name validity check for the branch name
Types of changes
Checklist: