search

vdemydiuk / mtapi

MetaTrader API (terminal bridge)
http://mtapi4.net/
MIT License
511 stars 276 forks source link

update code to C# 7 and vs2019, minor refactor #263

Closed sanikolov closed 2 years ago

sanikolov commented 2 years ago
  1. update code to C# 7.0
  2. create a vs2019 sln and upgrade toolset from 141 to 142
  3. VS 2017 did not compile properly, kept getting log4net reference errors - fixed
  4. Addressed code analyzer naming issues and other minor recommendations
  5. removed VS redistributable binaries from repo and updated chain loader - now the VS redistributable is downloaded and installed upon installation start
  6. fixed outdated attrs like src in wxs

All in all minor changes concerned with cosmetics rather than substance. Looking forward to the .NET Core port that is pending to get merged.

lazou commented 2 years ago

Hi @sanikolov , is it possible that you used a different line ending (LF instead of CRLF)? The diff look like this is the case.

eabase commented 2 years ago

@sanikolov

Please redo your PR, because it's impossible to accept a PR when it looks like every single line has changed.

You changed line ending or file encoding without considering this effect and any time line endings change, you need to do that in a completely separate PR, that doesn't get squashed.

⚠️ In addition there are actual code fixes mixed in with formatting, mixed in with changing case standard from none, Camel to Capitals without explanation, and what effect this may have on DLL dependencies such as python pythonet and ctypes usage. I'm pretty confident this might (❓ ) break all of those, since AFAICR their name changes are propagated into the DLL! ❗
(I haven't tested this, so if I am wrong I apologize in advance, but this remain to to be clarified.)

This is probably related to the char set used by @vdemydiuk is some of his files. They should all be UTF-8 and not any other exotic UTF-16LE/BE etc.