Code review comment for lp:~jeremywootten/pantheon-files/fix-network-browsing

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

Initial responses to Carl's review:
1) Should we not restore network folders?
2) The "fatal" errors are not really fatal. I'll see if I can quiet them. Do we need a message in the user interface?
3) This is outside the scope of the branch -and also happens on other File Managers (Thunar, Nautilus). Do we need "Properties" dialog for network locations?
4) Do we need "Properties" dialog for network locations?
5) Will not show "Open in Terminal" for network locations.
6) and 7) and 8) Server connection dialog is outside scope of branch
9) Could you give more explanation? I am not sure what steps you carried out.
10a) This is an issue with the connect to server dialog - outside scope of branch.
10b) Could you give more explanation? I am not sure what steps you carried out.
11) Could you give more explanation? Do you mean it doesn't appear in sidebar (in the right place)?
12) Warnings are due to failed attempt to connect a FileMonitor to a network folder that does not support it. It is expected to fail under these circumstances, so the warning is not really required. I'll change to debug.
13) This needs investigating - if you can identify which circumstances cause it please let me know. I'll try to reproduce. I think I saw it in earlier versions of the branch but not recently.
14) Do you mean servers are not remembered in the "Connect to Server" dialog? This is outside the scope of the branch
15) This suddenly stopped working for me too, seemingly after connecting a Windows machine to the network. But it was the same with Thunar and Nautilus. Fixed it by changing the name resolve order in etc/samba/samb.conf [global] section to "name resolve order = bcast host lmhosts wins". It appears to be a quirk/bug in Samba itself.
16) Confirmed. This is because "Windows Network" is not a real folder but the location bar thinks it is. I'll inhibit bookmarking of pseudo folders for now.

« Back to merge proposal