-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
using TC library API library wrapper... #20
Comments
Anything strongly typed gets my +1 :) |
Under the covers, it's still going over HTTP. It is always loosely typed somewhere in the chain. Adding a dependency on TeamCitySharp is just hiding the loose typing somewhere further down the stack. |
There is always a schema :), either explicit (e.g. a client library) or implicit (directly in the code). I see it We use strongly typed View Models in MVC because parsing HTTP is not where we want to spent our time. |
The related PR, #19, contains 0 changes to HTTP parsing logic whilst still implementing new behavior. It is existing, working code. What's the value of changing it? |
You are right the code works just fine. I will let @RaphHaddad explain his own reasons. From my perspective it is about the cost of maintenance. When I wanted to extend TeamFlash I had to check TC docs + run the app to know how certain information flows from the server to the code. With strongly typed client this work is done for me by the compiler. |
In addition to @pawelpabich comments. I believe that if we want to add features like excluding/including projects and branches etc, it would be wise to have some statically typed classes. A bit of magic is OK. Too much isn't. :P I understand your concern @tathamoddie in terms of the TC API being loosely typed down the chain. However, having that first level of the chain defined as a schema helps a lot. |
..rather than dynamic magic calls without static classes.
What do you guys think of something like this?
https://github.com/stack72/TeamCitySharp
It would make it easier to adapt the project for extra and more complicated options
The text was updated successfully, but these errors were encountered: