Closed Patowhiz closed 2 years ago
@Patowhiz Thanks for tackling this, should this PR reference issue #7 ?
@lloyddewit this is now ready for review.
As mentioned in issue #5 I didn't parameterise function GetDynamicTranslation()
because I still don't understand the intention of the sql
statement. I understand it has something to do with R-Instat product. So I left it the way it is. If we happen to know the sql
intentions I can adopt it to use GetTranslations()
that I have added.
@lloyddewit the other questions that I had which is can we make this library to ship with its dependencies when used by the products, such that the products don't need to install the dependencies used by it. The advantage will be avoiding any future bugs that may be introduced by different versions of the dependencies.
If the above is possible I'll suggest we take that option.
@lloyddewit the other questions that I had which is can we make this library to ship with its dependencies when used by the products, such that the products don't need to install the dependencies used by it. The advantage will be avoiding any future bugs that may be introduced by different versions of the dependencies.
If the above is possible I'll suggest we take that option.
In theory, it's possible to create a NuGet package that contains all the dependencies. Let me investigate this.
@lloyddewit keep on with the suggestions. I'm usually terrible with code comments. Thanks.
@lloyddewit I've made changes requested. Thanks.
Fixes issue #6 and #7 Separated single class to 3 classes. Generalised the functions to work with multiple products. Added newtonsoft .net JSON nuget package for reading and writing CrowdIn JSON files.