Tool: Pyflakes Processed Files: rbtools/clients/git.py Tool: PEP8 Style Checker Processed Files: rbtools/clients/git.py
Don't give "dirty" warning only because of dirty submodules
Review Request #8627 — Created Jan. 17, 2017 and submitted
Ignore changes in submodules working directories when checking for pending
changes: it is common to have some untracked files in the submodules (e.g.
those produced during the build) and sometimes some existing files could be
modified as well.
Also, as diff doesn't include changes to the submodules in any case, it seems
logical to exclude them from the dirty check too.
I'm quite sure that the current behaviour is not ideal because a submodule with some untracked files in it really shouldn't result in
WARNING: Your working directory is not clean. Any changes which have not been committed to a branch will not be included in your review request.
as it currently does and so I think it's clear that
--ignore-submodulesoption should be used. However I'm not sure if my patch is ideal neither: I can see arguments both in favour of changing the value of this option to
untracked(because a really dirty submodule could contain some changes that were meant to be committed and forgotten) and
all(because it would be consistent to ignore the submodules completely for consistency with
make_diff). So while I believe that using
dirtyhere is the happy middle ground, I would totally understand if you changed it to something else. But, again, IMO at the very least
--ignore-submodules=untrackedshould be used because it's pretty common to have untracked files in submodules and the warning is just distracting and teaches users to simply ignore it.
To be honest, not much: just reran the same
rbt postcommand that warned me about working directory not being clean before and confirmed that the warning didn't happen any more.