Include Ignored Options #1328
Include Ignored Options #1328
Conversation
|
Looks like you want commit coming in on the vnext branch. Don't see a way to edit target branch let me know if I just need to close this and start a new request with the same code. |
|
Hi @paulgmiller ! Thanks for the submission. I have a few comments that I will add inline, but we do actually want this to target the master branch. This is a relatively recent change, we used to use vNext. Where did you see the guidance to open pull requests against vNext? I need to update that. 😀 Including ignored seems like an odd default. I don't have the historical context to know why It would be a bit of a breaking change to change the default, but properly documented, it might be the Right Thing. @carlosmn ? |
| GitStatusOptionFlags.IncludeUntracked | | ||
| GitStatusOptionFlags.RecurseUntrackedDirs, | ||
| }; | ||
|
|
||
| if (options.IncludeIgnored ) |
There was a problem hiding this comment.
Minor nit: there's an additional space after options.IncludeIgnored...
|
Vnext branch is refrenced in https://github.com/libgit2/libgit2sharp/wiki under contributing items 1 and 7. Fixing up minor comments. Might be a good idea to make the default change separate so it can be reverted if need be. I'll run a git blame to try and see who introduced it but I obviously defer to you guys there. |
|
Seems like it came in as the default in 974ad99 when you changed to track renames and moved away from git_status_foreach, |
…s can opt out for perf reasons. Add unittests for that status option.
|
Anything else I can do here or are we waiting on @carlosmn's input on the default. |
|
Including ignored is probably to do with rename detection or somesuch. I don't like having that as default, but let's not change it for now. |
|
Sounds fine. I think that this is a crap default, and I think that it was my fault, and we should change it, but I think that it probably does make sense to change this in a separate PR. (@paulgmiller are you interested in doing that?) |
|
Maybe let me see how many UTs break. Unfortunately the product I noticed this in is considering moving to git.exe parsing rather than continuing to call libgit2sharp. Could have a longer conversation about why if you're interested |
Feel free to drop by our slack if you want to chat. |
We have some repos that produce thousands of ignored files so retrieve status takes 20 minute on them if we include ignore files vs a couple of seconds if we don't. There fore we were hoping we could make including ignored files optional.