search

vdemydiuk / mtapi

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

Update .gitignore to allow Release DLL's #233

Closed eabase closed 3 years ago

eabase commented 3 years ago
lazou commented 3 years ago

From my point of view, release or debug builds (output after compiling) should never be part of a repository. They should be always provided via official release artifacts. So I would not recommend this change since it is bad practice in software development.

eabase commented 3 years ago

@lazou

... They should be always provided via official release artifacts.

That is exactly why I made this! It would allow the official releases DLL's to be individually available here.

So I would not recommend this change since it is bad practice in software development.

That is just your opinion, and certainly not everyone else's, who are also "developing". So I don't understand what problem you have with this. In addition the DLL's are quite small.

KptKuck commented 3 years ago

@eabase The better way is to learn how to compile single DLLs. I see it like lazou

lazou commented 3 years ago

@eabase It is only my opinion based on years of experience. But since I do not own this repository, I gave only a recommendation. In the end, @vdemydiuk has to decide. There is also an official guide from github, how to manage releases: https://docs.github.com/en/free-pro-team@latest/github/administering-a-repository/managing-releases-in-a-repository

eabase commented 3 years ago

Although I respect your experience, and generally agree with you. In this case I do not, because the installer is not able to find the Terminal folders on it's own, so the user must locate, find and move the DLL's manually, which defeat (most of) the purpose of the installer, in the first place. In addition, you may not want to have to put small project DLL's in the System PATH, as then every damn thing you install (which require homemade DLL's) would need to be added there as well.

@lazou That link does not mention DLLs at all... Dead end and dead argument.

PS. I'm not here to argue with you guys, but just sent a PR for something that helped me and that I know other more technical user would consider useful and helpful. I really don't care your opinions about what is "right" and "appropriate". It wasn't for me.

lazou commented 3 years ago

Why are you so aggressive? I only want to add some information which maybe will bring us all to a better or more suitable solution. It was not my intention to bother you at all. Neither to argue that your PR is bullshit (which is not the case!). I have no real insight in the installer, so you are more familiar with it. But maybe there is a better way to fit your needs and also take the recommendation into account. Development is always based on teamwork, nobody knows everything. Therefore, please be open to others opinions and discuss carefully. I respect your opinion here, but this does not mean that I will have the same.

vdemydiuk commented 3 years ago

@eabase sorry, but I have to reject your PR. I agree with @lazou - pushing compiled DLLs into repo is bad practices. The main reason - DLLs must be always in actual state with source code. That means I must upload new DLLs with every commits and other users must pull binaries every time. This is not very useful. But agree with @eabase that some users just want to download ready DLLs without building the project. In that case I will add zip of binaries into releases which will contains all DLLs.