Subject: Re: [X2Go-Dev] Bug#1469: Patch!

From: Melroy van den Berg <[email-masked]>
Date: Tue, 19 May 2020 20:00:40 +0000

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 <[email-masked]> 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: [email-masked], http://das-netzwerkteam.de