Don't give "dirty" warning only because of dirty submodules
Review Request #8627 — Created Jan. 17, 2017 and submitted — Latest diff uploaded
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-submodules
option 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 tountracked
(because a really dirty submodule could contain some changes that were meant to be committed and forgotten) andall
(because it would be consistent to ignore the submodules completely for consistency withmake_diff
). So while I believe that usingdirty
here 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=untracked
should 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 post
command that warned me about working directory not being clean before and confirmed that the warning didn't happen any more.