Offer #1013 remove force from nlu

[offerakrabi]

removed all the force parameters from the nlu objects
also unexposed factory

[erezbi]

What is 'Init' that you added? How did it work before you exposed it?

Also, what testing did you do on this?

[offerakrabi]

@erezbi
I just moved the function from the boilerplate (that uses the factory) to the sdk, you run it by calling init.
I can change the name of the function to initEngines maybe.

I just tried to run it a few times - We can run more tests if we want.

[erezbi]

does this need a change in the boilerplate then, to accompany it?

[offerakrabi]

yes, I need to update the example nlu engine as well

[erezbi]

So this will be a breaking change for all skills? they'll need to change something in the skills because of this change?

[offerakrabi]

no, this parameter isn't used.
any skill using an outdated sdk won't be effected.
and in the boilerplate it is present and not used (even says deprecated in the documentation)
so again, shouldn't be any problems for outdated boilerplates

[erezbi]

I was not referring to the 'force' parameter; i was referring to the fact that 'factory' was exposed, and some code depended on this. now it won't be exposed, so some code will need to change to use 'init' instead of 'factory'. is the code that needs to change in the skill BP? I am guessing it is because I didn't see it being changed in this commit.

or, did you forget to change some other line - that used factory instead of 'init'?

what am I missing here?

[offerakrabi]

Yes, you are correct..
This change moves some of the code from the boilerplate to the sdk, thus there are some changes in the boilerplate as well (already made them)
https://github.com/offerakrabi/ExpertiseBoilerPlateRemote/tree/offer_%231013_remove_foce_from_nlu
I didn't make a pull request for this yet.

[erezbi]

is this breaking change needed? should we wait with this till we have a better reason for a breaking change?

mind you, it would be even worse to break things after GA.

[erezbi]

Also, the 'force' isn't a breaking change.
We can do it in a different PR, only for this.