What would it take for someone to become a binding reviewer?

Development Misc
13

I will throw in my 2 cents here and answer the thread topic from another point of view.

“What would it take for someone to become a binding reviewer?”

I think the question is a bit too common.
As we have read in some posts already it will require a bigger effort to become a full code reviewer, with push access to the repository.
But that doesn’t prevent anyone from reviewing Pull Request, even if one is not that familiar with java and the code of that special binding.

When i have enough spare time, i am helping out with readme reviews in the openhab2-addons repo.
Mostly i can’t tell too much about the content, but i am familiar with how we build the docs and can throw in suggestions for the writing style and how the readme is written.

If you have the binding hardware, you can of course test the binding practical and give feedback.
Reviewers will be thankful for your help, since it makes the review easier and may speed up the review process.

It’s already possible to downlad and install bindings from pull requests, but the installation process may be a bit too advanced from what i read here.
Marketplace was a nice solution for this problem, so we should reach out for a valid replacement.
But as david said, a new binding should at least have some rough review for malicous code before getting into a testing phase.

14

Matt: your idea is excellent and I think it needs split out into another thread.

15

Really good point here folks, reviewing the bindings begins with logging on to git (you need an account) and digging in. There is nothing stopping anyone from making review comments. The developers respond to any help. If you think you know java well enough to begin learning to review bindings, get involved, learn the basics, follow along.
When I wanted to help with the docs, Jerome taught and encouraged me. He (and others) reviewed my changes, suggested improvements, taught me the platform and got me thru the process.

16

Feel free to copy any of my text and post into a new thread, I don’t mind… I prefer to spend the 1-2 hours I have spare coding and not typing on a forum.
I don’t really see it as my idea as this issue of too many bindings building up in the que has been going on for years and the Marketplace was a solution of sorts put in place. See link below.

David is creating a new UI and I am sure he will have some ideas on how to implement something that is an improvement over what we have now.

1 Like
17

Believe it or not but all pieces except two are done to solve this scaling problem and enable us to have a staging repo or alternatively to publish bindings that are added as pull requests.

  • A new buildsystem that builds up on standard Java OSGi ecosystem
    • We have one type of repositories now (maven) and no more Eclipse p2. That always complicated stuff with having multiple addon repos.
    • Any maven plugin can be used. We don’t need to consider complicated tycho lifecycle problems.
    • We have BOMs now (Maven bill of materials). So a staging addon repo’s buildsystem file is only about 50 lines including automatic upload to bintray, because we can reference those BOMs from other repos.
    • A better way to handle 3rd party dependencies
  • I have rewritten the extensionservice part of openHab core to allow users to add maven and karaf repositories and I will propose to add a bintray location where pull request builds are located. It also supports a maintenance-status for extensions (abandoned, beta, release). I like the idea of a rating and comment system. That is not yet included but would also require openhabCould to be installed.

What is currently missing is:

  • The continuous integration service (travis-ci) must be instructed to
    • build a new-binding pull request with very strict static code checker settings. A failed build will motivate the developer to fix all the issues that we at the moment need to point out manually.
    • upload a successful new-binding kar file (karaf feature file) to a staging karaf repository on bintray.
  • A community review procedure. Personally I think a 5000 lines new binding cannot be reviewed correctly by us maintainers. The community must help with functional testing. Maintainers will only read over and help with choosing the right openHAB pattern and resource usage (that is mostly done wrong unfortunately).

And yes my paper ui design study already supports managing multiple repositories. That will work as soon as OH core can accept my extensionservice changes.

Cheers, David

3 Likes
18

IMHO the OP was intended in such direction. I’d be among the first to step forward doing such community reviews.

19

With procedure I mean: A way that you can tell in an automatic or semi automatic fashion that the binding works as intended. So that maintainers only start to perform a complete review (can take several hours) when the binding is actually working and deemed useful by the community.

I imagine that we have a forum section where a bot adds new binding threads for people to discuss them and add an “approve” tag or something which results in the github pull request to be marked as “community approved”.

20

That’s indeed what I meant in the OP. I’m looking for some way to spread the burden among some of the “power users” will many not have the technical experience in OH and perhaps even in Java to do the full review.

My intent was never to replace the maintainers as having final approval so I done think we would need to give these reviewers any more permissions on the repo beyond commenting. With the “approved by community” tag or the like the maintainers well know they don’t have to look very closely at certain aspects of the binding.

As to the staging repo/alternative repo for bindings, that’s a great idea. The whole ecosystem will benefit if it’s easier for years to find and install bindings that have not yet been reviewed through the UIs.

The only thing I would ask as this is our together is to consider also that we will need a way to distribute Rules Templates as well. It would be nice if we can use the same mechanism.

21

The extensionservice originally written by Kai considered all types of extensions including rule templates. Bindings can also bundle rule templates.

What we probably need is a new directory on openhab2-addons for ruletemplates (or a separate repo) and an extensionservice provider that gets new rule templates periodically directly from the github repo.

1 Like
22

I think it is time to split automation from core, and have a repo for automation engines, script engines, and rule templates. Potentially helper libraries too, but most/all of the functionality they provide will be getting absorbed into a scripting API.

I’ve been working on incorporating this into the Jython addon, but I’ve had to put that down to focus on the helper libraries. I will definitely need your help with this addon!

1 Like
23

I guess you are proposing that because Core is slow in merging automation engine changes.
I’d say we convince Core maintainers to allow us full, unrestricted, unquestioned access to the “org.openhab.automation” bundle namespace (it’s fortunately still marked as experimential).

It takes too long to wait for Markus AND Kai’s approval, because neither of them dares to merge at the moment without the other (that’s my personal feeling, might be wrong?!).

Script “engines” (the java part is what you mean I guess?) should stay in core is my gut feeling. And I’d love to contribute all my written automation module handlers, if I’d know that review speed is not measured in weeks on that repo.

Helper libraries and the resulting scripting API should be on its own repo. Did you get a repo for that purpose already?

1 Like
24

Sorry for reviving, but what is the conclusion of this thread?

I reviewed the PR for the SMHI binding, but my review doesn’t count of course, because I’m not a maintainer. I program Java since 16 years and do Java code reviews as a software architect at work. My binding contribution history is not as long (one binding (13k lines), which is waiting to be reviewed).

What do I have to do to become a code reviewer?

1 Like
25

Hi Fabian,

Your help will be appreciated. I guess you are already familiar with our guidelines to become a maintainer. I would suggest you head over to the add-ons repository and start reviewing other contributions. There are waiting a lot of new bindings. In the beginning you do not have to be a maintainer to help. Feel free to spend as much time as you can offer. Once the staff recognizes your good work one has to nominate you as a potential candidate to joining the team. There will be a small internal voting.

Additionally it would make sense to get in touch with OHC too. Another option could be to volunteer for some special task to be done for OH3 (e.g. we want to get rid of dependency on org.apache.commons in all existing bindings). I know that is hard work but a task which has to be done by a maintainer as well.

Have fun. Don’t hesitate to ask further questions.
Christoph

3 Likes
26

I wish openHAB would communicate whether it seeks to be a platform or an application.

A platform enables decentralised third parties to autonomously design, develop, discover, distribute and deploy new capabilities. This requires prioritising features such as addon indexes, usage telemetry to surface quality proxies to intending users, security models (permissions, flagging, moderation, bytecode analysis), sandboxing (isolated logging, control flow instrumentation, operational metrics) etc.

An application enables end users to download a single centralised distribution and have a relatively reliable turnkey experience. But doing that requires a strong level of contribution gating that seems unrealistic for a vendor agnostic DIY solution, let alone one without any FTE employees or sponsors.

Increasingly open source projects communicate their ubiquity based on convenient community metrics such as number of plugins, lines of contributed code, quantity of contributors, number of downloads etc. Such metrics are trivially optimised by platforms while being comparatively difficult for those applications maintaining quality controls. In turn more accessible projects have a stronger marketing proposition, in turn leading to accelerated adoption, contributors and competitive position (uncorrelated to underlying quality).

I’ve had 6 recent experiences with the addon ecosystem that indicates to me it currently struggles:

  1. I purchased some hardware and found the official addon broken, so I located an earlier bug report, made a small fix and submitted a PR. While the PR was promptly reviewed, I was asked to make substantial changes and the overwhelming majority of those were unrelated to my small fix.
  2. I used an official addon and added a feature to it. It was circa 10 lines. The PR was promptly reviewed with LGTM but then no second reviewer ever dealt with it. Months later I closed the PR as the lack of the feature made it unusable and I switched to another addon.
  3. I use an unofficial addon supplied as a community JAR, but it broke due to hosted service provider changes. The author fixed it within hours and I wondered why it wasn’t an official addon given the apparent widespread use and responsive maintainer. I found an old post where he reported that it was once submitted but so many changes were sought that he didn’t proceed.
  4. I use an official addon despite there being a far more feature-rich unofficial version. The author never submitted it for a PR and has now disappeared without their useful work being built upon.
  5. I was trying to integrate some hardware and found there was a PR for it. But then I found it unreviewed, and even the author’s request here months later was met with “be patient”.
  6. Someone suggested writing an addon here on the forum. A few users chimed in to suggest it wasn’t that useful. The user cited another project that had a similar feature and expressed disillusionment and that they would go elsewhere.

Today I was working on a battery system integration and believe it would make a useful openHAB addon. Yet I have no intention of packaging it as such, as based on the above there is a high probability that it will either receive no review, no interest or require substantial PR modifications that I don’t have time for.

I hope that openHAB evolves into a platform that makes it easy to welcome and channel the enthusiasm of prospective addon developers. There are ample precedents (app stores, browser plugin directories, operating system package managers etc) on how to achieve this. Just appointing more reviewers is fine, but it won’t change the experiences of #1, #3, #4 and #6 above, as the review process is aiming to enforce a quality benchmark that makes the bar so very high. A platform, on the other hand, would capture all the energies put into every example above and leave everybody - irrespective of their enthusiasm or available time - able to constructively participate and share their work.

4 Likes
27

The platform you seek was and, for now, remains the Eclipse IoT Marketplace (you can find the plugin to install it under the Misc tab of PaperUI). This was envisioned as a way for people to make available add-ons (and eventually reusable rules and the like) without submitting it to openHAB for review for what ever desired reason (some don’t like the review process, others don’t like the licensing requirements, etc).

Instructions for adding an add-on to the marketplace can be found at https://marketplace.eclipse.org/addcontent.

From the user’s perspective the add-ons show up in the list the same as the official ones and can be installed in all the same ways as the official ones.

I believe an alternative marketplace is in work for OH 3.I don’t know any details.

And of course, one can manually install add-ons, as you indicated.

Support for a platform is there and used by many. I’m not sure what else is required. There are at least two alternatives to make add-ons available for those who do not want to go through OH’s approval.

1 Like
28

Not sure either, but I don’t believe there is…
This is a legitimate concern however, of course, bindings are only one of these kinds of add-ons you would want to “sideload” because the official distribution corresponding to your runtime release doesn’t meet your needs. Other kinds could be rule templates, UI widgets, and more.

I experimented with HABPanel a community-curated gallery of widgets, which uses this forum’s API to present options and download attachments, as well as being able to download from GitHub repositories when needed. It gave good results, but I’m not sure if it would be appropriate to expand this approach to the kinds of add-ons which would be a real security threat if added while not properly reviewed - like bindings. On the other hand: do we really have the bandwidth to properly review what’s being contributed in a timely fashion?

29

This is getting a bit off topic :slight_smile:, but there is, as far as I know, not yet any active work done other then some ideas that can be found here: https://github.com/openhab/openhab-addons/issues/6673

2 Likes
30

sorry, I know it’s a common occurrence :wink:
Thanks for the very interesting link, was not aware of it!

31

No, and we shouldn’t even try that. Instead of making core contributors getting over each and every line of every addon which is intended to be integrated I believe it is better to focus on making a quick feedback loop from end users back to contributors. If binding passes a static code analysis it is a valid candidate to make it available for installation with appropriate “unverified”, “experimental”, “preview” flags.
If you think that forum and marketplace enables that, you’re wrong. First time contributor might not know what marketplace is. Marketplace is not targeted toward openHAB deployments but Eclipse project which is now gone. It is also beyond control of foundation. Forcing people to re-publish their contribution elsewhere so end users could play with their work is slow.
Automation is a key. What @Ben said is very accurate - if we could automatically build PRs, publish and brodcast them as experimental features and post back uncaught exceptions from given binding back to its author quality improvement would be faster than current review. :slight_smile: The @NonNullByDefault would become optional. :wink:

Personally I do not understand why openHAB try to impose so many quality metrics on addons which in some cases get used by very few users. People who write their contribution first time need endorphins to keep their enthusiasm up. Waiting for weeks in the review queue or receiving number of framework related philosophical pointers kills that.
If it doesn’t work - then it doesn’t work. People in interest must communicate. Shielding contributor from users will not work anyway. Most of issues will be assigned back to author anyway, fairly often authors will be reached over forums.

That time for reviews would be spent better on improving developer documentation. This was requested in other thread few days ago. Explaining APIs which are invisible for first time contributors is better than training them via reviews.

5 Likes
32

I think all necessary technical preconditions are already met.

But the expections and the available alternatives could be managed better. Give the user an overview:

If you want your add-on to be part of the official distribution, there are some quality gates. Expect n review findings per 1000 loc on average. Expect the first review after n weeks.
If that isn’t what you expected, you can release your add-on via the marketplace without any barrier. [Link to how easily it is to install a marketplace add-on]