Jeff King <peff@xxxxxxxx> writes: >> " git rm --cached %s\n" >> "\n" >> -"See \"git help submodule\" for more information." >> +"See \"git help submodule\" for more information.\n" >> +"\n" >> +"If you cannot use submodules, you may bypass this check with:\n" >> +"\n" >> +" git add --no-warn-embedded-repo %s\n" >> ); > > I was a little surprised by this hunk, but I guess if we are going to > block the user's operation from completing, we might want to tell them > how to get around it. But it seems odd to me that the instructions to > "git rm --cached" the submodule remain. If this situation is now an > error and not a warning, there is nothing to roll back from the index, > since we will have bailed before writing it. > > If we are going to start recommending --no-warn-embedded-repo here, > would we want to promote it from being OPT_HIDDEN_BOOL()? We do document > it in the manpage, but just omit it from the "-h" output, since it > should be rarely used. Maybe it is OK to stay that way; you don't need > it until you run into this situation, at which point the advice > hopefully has guided you in the right direction. If we are keeping the escape hatch, it would make sense to actually use that escape hatch to protect existing "git add" with that, instead of turning them into "git submodule add" and then adjust the tests for the consequences (i.e. "submodule add" does more than what "git add [--no-warn-embedded-repo]" would), at least for these tests in [3,4,5/6]. Also I do not think it is too late for a more natural UI, e.g. "--allow-embedded-repo=[yes/no/warn]", to deprecate the "--[no-]warn-*" option.