openhab-scripters / openhab-helper-libraries

Scripts and modules for use with openHAB
Eclipse Public License 1.0
88 stars 69 forks source link

[utils] Rename postUpdateCheckFirst and sendCommandCheckFirst #186

Closed CrazyIvan359 closed 4 years ago

CrazyIvan359 commented 4 years ago

Updates names of postUpdateCheckFirst and sendCommandCheckFirst to postUpdateIfDifferent and sendCommandIfDifferent. The old names are not very clear about the use of the function. The original names have been preserved as aliases for backwards compatibility.

Fixes #185

Signed-off-by: Mike Murton 6764025+CrazyIvan359@users.noreply.github.com

5iver commented 4 years ago

I almost merged, but had a thought. What if we used this as an opportunity to snake_case them?

CrazyIvan359 commented 4 years ago

Didn't we decide we should be going the other way? The goal is to merge most of these libraries into oh core or an addon, oh is all camelCase.

5iver commented 4 years ago

Didn't we decide we should be going the other way?

No... if anything, everything should be converted to snake_case to help prevent name conflicts.

CrazyIvan359 commented 4 years ago

While I agree with that, wouldn't that be bizarre after integration with oh? Having oh in camelCase and our addon in snake_case.

5iver commented 4 years ago

Why do you think it would be bizarre? Once the Scripting API is in place, everything will be in mixedCase, except for anything still left in the libraries. I think it would help to differentiate what you are using.

CrazyIvan359 commented 4 years ago

Oh I see, you don't mean to avoid naming conflicts with OH as it is now, you mean between the HLs as they are now and the forthcoming API!

5iver commented 4 years ago

Correct. There could potentially be other things in OH or Java too, but the Scripting API would be a major source of name conflicts.

CrazyIvan359 commented 4 years ago

Yes, now I understand. I will update the names

besynnerlig commented 4 years ago

Even though the original names have been preserved as aliases for backwards compatibility, maybe we should consider to rework all the included libraries and examples (e.g. only ideAlarm a.f.a.i.k) to use the new names.

5iver commented 4 years ago

Once this is merged, I'll update any usage in the repo with new function names...

https://github.com/openhab-scripters/openhab-helper-libraries/search?q=postupdatecheckfirst&unscoped_q=postupdatecheckfirst https://github.com/openhab-scripters/openhab-helper-libraries/search?q=sendcommandcheckfirst&unscoped_q=sendcommandcheckfirst

CrazyIvan359 commented 4 years ago

@openhab-5iver shouldn't that renaming happen in this PR?

5iver commented 4 years ago

Yes, but I didn't want to make work for you. But you're going to need to rebase anyhow, so might as well get it in.

CrazyIvan359 commented 4 years ago

Rebase seems like a lot of trouble for a small PR like this. Just make the changes to master and close this PR with a reference to the commit instead.