Closed mjwsteenbergen closed 5 years ago
Hi, first of all thanks for the improvements on the library and sorry for not reviewing them earlier. To reference all your points:
I agree with switch to .netstandard 2.0, that's fine and wanted
Tests are fine and welcome
Easier setup, let's say it is - but there are some things in your readme which are not recommended, for example, app should use GeneratePSK method to generate its own PSK and not to use default one. Also, for inexperienced devs I think it is easier to have an 'example' project which is showing them how they should use the library.
ApiLibs - I think it's possibilities are probably way beyond our scope
JSON ok
Nuget spec in csproj is also fine but the Devops build procedure which is configured needs to be reconfigured
Smaller namespaces, generally namespace length shouldn't be a problem, anywhere. Basically I don't mind changing the namespace but not because of shortening and not just to an official product name. I don't want people to think that this is an official nuget by the name so I'd prefer the old namespace for now
Tradfri, with small f, is fine with me and you are right about it actually
As I said, I think we probably don't use more than 5% of ApiLibs but currently let's say it is not a big problem
I think we should have the example project - from my experience it is more useful than writing a plenty of documentation for the beginners
Regarding this we should consider couple of options, that it should be a new nuget (with new name, for example 'Tomidix.Dotnet.Standard.TradfriLibrary' or 'Tomidix.DotnetStandard.TradfriLibrary' where we can just say that the old version is obsolete and not maintained anymore but stays alive on nuget.org feed
First of all I'd like if you can implement observable part which should be in conflicting files. After that I can complete a pull request and add you as collaborator so we can work together on all of our points.
Thanks again for your PR submit.
First of all, thank you for the work you've put in so far. You've done great work.
Changes:
.csproj
(reference)Changes you might not like:
Tomidix.CSharpTradFriLibrary
is now justTradfri
TradFri
is nowTradfri
. I am used to camelcase andTradFri
looks to me like it is two words Trad and FriAny feedback welcome.