Hi,
Well.. it was not unnecessary. The code was actually unreadable, wrong indentations and inconsistent throughout the whole file. I needed to execute this auto-formatter, before I could even begin to understand the code.
In fact, I urge you to execute this on the whole archive. And keep the code quality high via a CI pipeline or something.
I can understand it makes it hard to read. I also fixed other some issues like this double error message during login and all the debug statements. What would be your approach? I think it's still wise to auto-format the code in consistent matter.
Regards, Melroy van den Berg
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ Op dinsdag, mei 19, 2020 9:48 PM, Mike Gabriel <mike.gabriel@das-netzwerkteam.de> schreef:
Hi Melroy,
On Di 19 Mai 2020 03:59:16 CEST, Melroy van den Berg wrote:
Hi, I did some refactoring in the sshmasterconnection.cpp file. I think > it can definitely use some clean-up and further splitting functions > and even into multiple files eventually. I cloned the master branch of code.x2go.org/x2goclient repo. The changes I applied are in the attachment of this mail (a git > patch file). I think closing previous sessions and connection solved > my strange SSH connection is resulting into Socket error: No such > file or directory". Although X2Go client is calling libssh > calls (libssh version > currently in use runtime on my host is: 0.7.0). For some reason this vague error message popped-up by libssh, > properly a result of previous session being created and not > correctly closed. And/or not closed in the right order (/missing API > calls), like a good example: ssh_channel_send_eof(channel); ssh_channel_close(channel); ssh_channel_free(channel); And I created yet again a new YT video showing my result: https://youtu.be/vmASLJq0CKM (including some brief explanation what I did so far, I also ran an > formatted on the code, because it was very very hard to read with > all the wrong indents) I also made another video showing yet another bug I found regarding > the SSH tunneling (during some heavy testing), the only solution was > to restart the whole docker image: https://youtu.be/xEpIyo84fWc (I think this issue is unrelated, but > maybe wise to take serious as well) Again see attachment for the patch (branched from master). Regards, Melroy van den Berg
Thanks for working on X2Go Client. With your patch, there is a problem though. It is barely unreviewable, because you have so many changes (also white space changes) in one patch file.
Please clone the X2Go Client Git repo, add atomic changes with good commit messages. Keep white-space changes and technical changes fully separate (ideally: technical changes first, white-space changes last).
Then export your patches using 'git format-patch <last-official-commit>..HEAD' and send us the set of patches instead this one bulk commit/patch.
Thanks for your efforts!!!
Mike
DAS-NETZWERKTEAM c\o Technik- und Ökologiezentrum Eckernförde Mike Gabriel, Marienthaler Str. 17, 24340 Eckernförde mobile: +49 (1520) 1976 148 landline: +49 (4351) 850 8940
GnuPG Fingerprint: 9BFB AEE8 6C0A A5FF BF22 0782 9AF4 6B30 2577 1B31 mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de