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
_______________________________________________
x2go-dev mailing list
x2go-dev@lists.x2go.org
https://lists.x2go.org/listinfo/x2go-dev

Reply via email to