Comment 16 for bug 1659229

Revision history for this message
Bryce Harrington (bryce) wrote :

I did some review work on the patch, but it feels like it needs further attention. Unfortunately, this was already pretty late in the freeze, and now we're past schedule on the release, so I think it's best if this patch be left for 0.92.2. It's a good solid patch though and definitely needs inclusion, but better if it not be rushed in.

I've broken the patch down into three sub-patches for easier review. The first is a mechanical refactoring with no logic changes to move routines from file.cpp to file-update.cpp. I picked a different function name. This also does some minor whitespace cleanup to try and improve consistency with surrounding code. I may have missed some whitespace but otherwise this patch is pretty good to go.

Second is the dialog replacement itself. I rebased this on top of the previous patch but didn't get a chance to do a line by line review; I've indicated some specific TODO's that need attention.

Third is the command line to go with the dialog. As I understand it, this needs expanded on to allow its use in no-gui mode. It also needs a man page entry, and I've left a few other TODO's.

Importantly, all the above also need to be landed on trunk. That could be done while waiting for 0.92.1 to be finished.