Hi Mike,
Thanks for your reply. Are are my thoughts.
Using (std::)endl is actually a bug
Ah, thanks for the heads-up! I think in that case we should in fact remove all the endl instead of adding it :).
Strictly speaking, though, this should not cause any issues
Why was I getting socket error then? I really hope everything got merged correctly now. Including my changes in the separate function.
Since all the changes together made my socket error from libssh disappear now.
This code path is for old libssh versions only (pre-0.8) and still works there, so I don't see any reason to change it.
I got deprecation warnings on this function/line while compiling. Why not merge it and remove build warnings?
The change in within the #else, maybe if you think it should be in the #if you can apply the change in that section (but I think in my case the else part got compiled).
Nevertheless, it's a deprecation that needs to be solved anyway.
Not changing return (...) vs. return ..., the actual change looks good though.
Why not changing the return(). This is wrong code, it's just a boolean! Bad practice, please don't it. Don't be afraid to change it.
A generic remark: I think X2Go is missing a good pipeline with testcases and other quality checks. Which also hopefully increases your *trust in the code* and enables refactoring as well.
I'm not afraid to refactor the code and clean-up the formatting, splitting functions and even into multiple files. If this all improve readability, debuggability and test-ability long-term.
That is why I raise a request to create a decent pipeline to allow the necessary changes in further improve code maturity and the needed changes to do so.
And maybe even a better diff tool to perform refactoring changes during review.
Any ideas or suggestions? I'm running a GitLab instance myself for example; which enables DevOps and CI/CD within all my projects.
Thanks once more!
Kind regards, Melroy van den Berg