Code review comment for lp:~gary-lasker/software-center/fix-makedirs-race-crashes

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks for updating the branch. Its fine now and I merged it into 5.2

I wrote a small unittest that does not exactly simulate the race, but gets close by patching os.path.exists() which will lead to calling the makedir code in any case so we can test that it does not crash for existing dirs and also propergate regular errors still.

As for the earlier comment, I did not want to fix the decimal vs octal issue during the merge because to me its not a trivial detail as it touched the test in such a way that the logic had to be inverted and it also touched two branches.

For small stuff I still do it (and you are always free to correct me here of course when I overdo it or make mistakes).

Small stuff to me is something like:
             LOG.warn("encountered non-writeable file, attempting to fix "
- "by deleting: %s",
- file_path)
+ "by deleting: %s" % file_path)
(not the "," vs the "%s") which is not really as issue as LOG will happily deal with the "," but the "%" is
more python. But no logic changed etc.

« Back to merge proposal