Code review comment for lp:~donadigo/pantheon-files/filechooser-module

Revision history for this message
Jeremy Wootten (jeremywootten) wrote :

The functionality and compile errors are now fixed - thanks.

There is more that could be done to eliminate duplicated code but I think that is best left to a separate future branch as it would conflict with other merge proposals affecting the pathbar.

The formatting is improved but there are still some issues:
1) FileChooseDialog.vala contains tabs. I advise you view your code in a text editor that displays tabs as an arrow so you can spot them easily.
2) FileChooserDialog.vala has misaligned braces in remove_gtk_widgets (), perhaps due to trying to reduce line length. I suggest that you move the bodies of the more deeply indented foreach blocks into separate functions to reduce line length and improve clarity. e.g.

 if (w1.get_name () == GTK_PATHBAR_PATH[1])
     remove_gtkpathbar1_children (w1);

3) GTK_FILTERCHOSSER_PATH is a mispelling of GTK_FILTERCHOOSER_PATH

4) There are files added to po/. These should be removed (bzr remove po/af.po etc) if they are not in trunk.

review: Needs Fixing

« Back to merge proposal