syncany-team team mailing list archive

[Philipp Heckel <philipp.heckel@xxxxxxxxx>]

Hello Vincent,

First of all: *Merry Christmas!!*

I'm answering here so everyone can take part in this discussion. I'll give
a lot of feedback and I'm hoping that this will help you/us for the GUI.
It's not a criticism, it's a way to make a good product :-) You're doing a
great job and I love what you've done so far. Also, I think the
websocket+SWT stuff really was a good decision!

@Everyone else: I'm referring to Vincent's work on the GUI here:
https://github.com/vwiencek/syncany

I actually fiddled a bit with the code and put some of my suggestions into
code changes. You don't have to take all of these changes, but its always
easier (for me) to have a little code to express yourself :-) -- Here's my
branch: https://github.com/binwiederhier/syncany/tree/gui - To give you an
impression what this looks like on Linux right now, here are some
screenshots: imgur.com/fUH6h1g,AcSuezQ,V2cJxHV,wEbdnT7,hzxqRYt  -- I made
the window a little bigger, that's why it looks a bit weird right now...

Now to my feedback!


*1. Executing 1.1.Executing with Gradle*: Running gradle runGui fails with
the following message. Any ideas? Running the printed command from
("--info") from the command line works perfectly, just running it from
gradle doesnt work ...

    :syncany-plugins:syncany-plugin-rest:processResources UP-TO-DATE
    :syncany-plugins:syncany-plugin-rest:classes
    :syncany-plugins:syncany-plugin-rest:jar
    :syncany-gui:runGui
    Error: Could not find or load main class
    :syncany-gui:runGui FAILED

    FAILURE: Build failed with an exception.

    * What went wrong:
    Execution failed for task ':syncany-gui:runGui'.
    > Process 'command '/usr/lib/jvm/java-7-openjdk-amd64/bin/java''
finished with non-zero exit value 1

*1.2. Eclipse execution*: It works from Eclipse, but I sometimes get the
following error in the "new Shell()" line: "Exception in thread "main"
org.eclipse.swt.SWTException: Invalid thread access"

*3. GUI + Daemon*
*3.1. **GUI<->daemon interaction:* When the GUI is started, do you think it
should try to start the daemon if it is not running? What if the GUI is
exited -- the daemon shouldn't stop then, right? Spitballing: Maybe, if the
GUI has started the daemon (it was not running), it should also stop it;
but if the daemon was running before, it should leave it running. Does that
make sense?
*3.3. Storing settings: *"properties.txt": You're storing the watched
folders in ~/.syncany/properties.txt; it's probably easier to do it like
the the lib is storing the config (ConfigTO, RepoTO, etc.)
*3.4. SWT dialogs: *You always do a shell.dispose(); when switching to a
new dialog. Is this best practice? Because it creates a flickering on Linux
when showing the new dialog. Also, if the old dialog was moved, the new one
always appears in the middle of the screen. I know that's kind of a big
change, but is it possible to switch the panels inside a dialog instead of
the dialog itself? Or is that not best practice with SWT?
*3.5. PluginGui -> Plugin panels:* I don't quite understand the rationale
behind the PluginGui class? It's never used, right? If you want to fill the
drop down for the available plugins, you could do that with "Reflections"
class and the method getSubTypesOf(PluginPanel) or something like that.
*3.6. Gradle module "syncany-plugins-gui"*: Is there a reason for this
module? Couldn't you just move it to the main gui package? Anything I'm not
seeing?
*3.7. E-mail dialog and e-mail in general: *The mockup I sent you included
the e-mail dialog and you implemented it like that. However, I don't see
that this should be part of the "default" selection. In fact, I don't think
there is a good default storage selection right now. Any ideas? I'd say the
only option right now is to simply show the drop down list of options.
That's why I think the mockups are helpful, because we can discuss before
implementing...
*3.8. "NewLocalFolders" dialog:* Right now we only support one folder per
repository (much like Dropbox), so there is no need for this dialog, right?
*3.9. Panel graphic (wizard left side):* If you want, you can use the panel
graphic I used back in 2011:
https://github.com/binwiederhier/syncany/tree/d8fa30f57e37355af8f70f0c2eda510ec43cbe74/syncany-assets/artwork/rendered/wizard
*3.10. Connect panel: *The "connect" panel should/must also allow users to
enter the full connection details, because you don't always have the same
conneciton details and want to share them with others, e.g. an FTP
syncany://-link includes the username and password of a user; if two users
have different usernames/passwords, it'd be stupid if they couldn't connect
to a repo.

*4. Tray Icon*
*4.1. Tray on Linux Mint: *The tray icon (yes, I'm obsessed!) on Linux
looks good only if it's not resized, so getImage() not getResizedImage()
*4.2. Tray menu updates: *I can see that there is a method updateTray(),
but its never used. I tried to call updateTray(update.getData()); and it
worked. I fixed it in my branch.

*5. Packages and Files*
*5.1. Package dependencies: *There are many packages with just 1-2 classes;
As I am not a fan of package dependencies, I think you should minimize them
by putting things together that belong together.
*5.2. syncany-util*: The syncany-util gradle sub project was a good idea.
However, I think it should only contain stuff that's actually needed by
more than just one module. An example would be the e-mail validation.
That's clearly just a GUI thing. As soon as it's needed in more than one
module, we can put it in syncany-util. Or is there a particular reason you
were putting it there?
*5.3. OS.java: *we both have operating system detection classes. We should
consolidate them. Mine is called EnvironmentUtil
*5.4. Leftover resources, classes and methods: * You have a lot of files
checked in that are referenced nowhere. Images, classes, methods. These
should be cleaned up.
*5.5. Code style: *Our coding styles are quite different. You'll see that I
moved stuff around a lot without changing the actual code. If possible,
it'd be great if you could try to adjust your style a bit to the rest of
the project. I know that's probably the hardest part, but it makes the
project look cleaner if the code looks similar in all classes. For some
things, you can use Ctrl+F.

I have more comments, but I think that's already a lot :)
And I should be getting to my parents' house anyway -- it's Christmas Eve
after all ... :-D

All the best,
Philipp

<http://pgp.mit.edu>