-
Notifications
You must be signed in to change notification settings - Fork 49
fix: ignore git submodules #661
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
Conversation
jfly
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.
Thanks for the contribution! Could you add a test?
97b3f19 to
811fe18
Compare
|
|
||
| // 160000 is the mode for submodules | ||
| if strings.HasPrefix(stage, "160000") { | ||
| g.scanner.Scan() |
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 don't understand why we need this here. Won't the for loop do the right thing when we continue?
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.
No, because this loop just calls g.scanner.Text(), and g.scanner.Scan() is called in the outer loop (the one we call nextFile in). If you want, I could refactor it so we call Scan in the nextFile function.
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.
Ohhh, got it. Your call, whatever you prefer.
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.
Can't get the refactor to work, so I guess we can just leave this here for now.
811fe18 to
c80829b
Compare
jfly
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.
Thanks for adding the test! LGTM
c80829b to
b96016b
Compare
Fixes #154.