Skip to content
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

Add Javascript libraries #120

Closed
5iver opened this issue May 21, 2019 · 7 comments · Fixed by #124
Closed

Add Javascript libraries #120

5iver opened this issue May 21, 2019 · 7 comments · Fixed by #124

Comments

@5iver
Copy link
Member

5iver commented May 21, 2019

Continuation of lewie/openhab2-javascript#4 now that #94 is complete

@lewie, how would you like to proceed? Would you like me to get things copied over first, using the new directory structure, or would you prefer to do that yourself? I've already added startup delay scripts, and I have a JS OSGI library to add too.

@Confectrician
Copy link

Throwing in my offer to help again, but lets wait for lewie to share his ideas. 🙂

@5iver 5iver pinned this issue May 21, 2019
@lewie
Copy link

lewie commented May 22, 2019

@openhab-5iver, It would help me if you made the first move, Thanks.

For the move, please use the version I already adapted for org.openhab.core:
https://github.com/lewie/openhab2-javascript/tree/smarthome-To-openhab.core

@Confectrician, don't let me stop you from developing. Get started!

@5iver
Copy link
Member Author

5iver commented May 22, 2019

@lewie, I'm ready to push a commit, but noticed your copyrights. Would you mind if this was changed to be more generic, as is used in the openHAB statement?

Copyright (c) 2019 Contributors to the openHAB Scripters project

Created PR #124. There is still testing to do, but I'd like to get it into the repo so that others can test too.

@lewie
Copy link

lewie commented May 23, 2019

@openhab-5iver, cool, LGTM! merge!
Thank You very much!

I can also correct the header after the move. But it would be nice if something about me as the original author would be preserved.

Sure, it's just only a script, but I'd like to know who to contact if I have any questions for example.

Not specifically related to my files, what do you think about headers?
Duty or voluntary, what is minimal required?
Is a simple license hint still missing for scripts?
Or at least what do we recommend to the developers?

The minimum that I would also require from example files:

/**
 * @copyright (c) 2019 Contributors to the openHAB Scripters project
 * @author Helmut Lehmeyer <[email protected]> - initial contribution.
 */

Later, when we have created order for the core classes, would I include a description and the origin in the class?
Like:

/**
 * @copyright (c) 2019 Contributors to the openHAB Scripters project
 * @license http://www.eclipse.org/legal/epl-2.0
 * @package openhab-helper-libraries - javascript
 * @author Helmut Lehmeyer <[email protected]> - initial contribution.
 *
 * @description: 
 * JSR223 JavaScript scripting extensions and helper classes
 * for simplifying the definition of rules with JavaScript Code used in openHAB 2.x.
 *
 */

@5iver
Copy link
Member Author

5iver commented May 23, 2019

I can also correct the header after the move. But it would be nice if something about me as the original author would be preserved.

Sure, it's just only a script, but I'd like to know who to contact if I have any questions for example.

Absolutely understood. I did a second commit that changed the copyright, so I could easily pull it if you wanted to keep it as is. The author tags are still there and I plan to get author tags in all the files. Documentation too, but that may need to be handled differently than with the Jython helper libraries (JythonHLs? 🙂). In my head, I tied that all in with adding versioning (#71), since it would mean touching all the files, but it would be best break this out into updating the files (copyright, license, author, version) and then use #71 for the actual version checking, notifying, etc.

Not specifically related to my files, what do you think about headers?
Duty or voluntary, what is minimal required?

IMO definitely needed and should be required. We can catch these in the PRs. An automated check could be added if/when the contributions increase.

Is a simple license hint still missing for scripts?
Or at least what do we recommend to the developers?

Yes. I think adopting the OH guidlines would be fitting (stock header with copyright and license, a main description with author tags, and documentation for classes and methods, etc. A formatter would be nice too (I think someone setup the OH one in VS Code).

I'll add some issues to continue this discussion. Could you please give me your thoughts on the suggestions over in #124? It would be easier/cleaner to just get those changes in now so that additional PRs aren't needed.

@besynnerlig
Copy link
Contributor

besynnerlig commented May 25, 2019

I believe that adding the javascript libraries has to do with that I'm now getting double startup delays. Not that another delay matters for me but ...

2019-05-25 15:19:44.919 [INFO ] [rt.internal.loader.ScriptFileWatcher] - Loading script 'javascript/core/000_startup_delay.js'
2019-05-25 15:19:45.012 [INFO ] [jsr223.javascript.core.startup_delay] - Checking for initialized context
2019-05-25 15:19:45.025 [INFO ] [jsr223.javascript.core.startup_delay] - Context initialized... waiting 30s before allowing scripts to load
2019-05-25 15:20:15.035 [INFO ] [jsr223.javascript.core.startup_delay] - Complete
2019-05-25 15:20:15.036 [INFO ] [rt.internal.loader.ScriptFileWatcher] - Loading script 'python/core/000_startup_delay.py'

@5iver
Copy link
Member Author

5iver commented May 25, 2019

I believe that adding the javascript libraries has to do with that I'm now getting double startup delays.

Yes... each language has a startup delay. I haven't thought of a way to prevent this.

@5iver 5iver unpinned this issue May 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants