Proposal: Remove Python & Go bindings to their own repositories

Gadgetoid commented 6 years ago

I don't believe having the Python & Go code here is a good idea, for many reasons but most prominent of all is that they add unnecessary burden to the maintainer, @jgarff, who must deal with PRs and issues against the language ports, in addition to the core C library.

It also sets a precedent that other language ports should be PR'd into this repository, rather than maintained by domain experts in their own repositories. This doesn't result in a very good separation of concerns. Although I appreciate it catches the edge-case where an issue with the core library might be reported against a specific port.

Do we really need to be funnelling fixes/issues for Python, Go, Mono - https://github.com/jgarff/rpi_ws281x/pull/58/files - and Rust - https://github.com/jgarff/rpi_ws281x/issues/265 - bindings for this library through one person?

I have the solution for Python- While it will almost certainly come across as self-serving to some, I propose that we instead focus on Pimoroni's hard fork of the Python bindings which currently lives here: https://github.com/pimoroni/rpi_ws281x-python

Why?

It's part of my job to maintain these bindings, so I have a vested interest in keeping on top of PRs and issues
These bindings are packaged and distributed via pypi, they're what you're installing if you type sudo pip install rpi_ws281x. There is no distribution for the Python code in this repository.
Right now we have duplication of effort- eg: https://github.com/pimoroni/rpi_ws281x-python/pull/1 versus: https://github.com/jgarff/rpi_ws281x/pull/266
I am still planning to distribute these bindings, and rpi_ws281x itself, as deb packages and submit them for inclusion in the Raspbian repository (I already do this for many Python libraries).

Right now this repository has multiple outstanding PRs and issues concerning the Python bindings: https://github.com/jgarff/rpi_ws281x/pull/258 https://github.com/jgarff/rpi_ws281x/pull/192 https://github.com/jgarff/rpi_ws281x/pull/163

https://github.com/jgarff/rpi_ws281x/issues/252 https://github.com/jgarff/rpi_ws281x/issues/247 https://github.com/jgarff/rpi_ws281x/issues/225

Frankly, this stuff is making this repository a confusing mess. So this is my impassioned petition to ask @jgarff to find a maintainer for the Go bindings in this library, and for us to work together to EOL the Go and Python bindings maintained here.

Feedback, particularly from the major contributors here is much appreciated;

Golang - @cpheps @pietrodn @darinkes @supcik
Mono - @zivillian
Rust - @jmurph2015

And in general @penfold42 and @jgarff

cpheps commented 6 years ago

I think this is a good idea, though I don't have the time to maintain the Go version of it. Developing against this was non-trivial for Go and it would be nice to see it broken out and given some proper love.

supcik commented 6 years ago

I agree. I already have a dedicated repo for go: https://github.com/supcik/ws2811. I can complete the repo with a README file and review/update the code if you want. Then we can just link our projects.

jgarff commented 6 years ago

I'm all in favor of having language specific maintainers. Great idea. I'm still however happy to maintain a central repo, with dedicated roles for various areas if that works for everyone. I'm not sure how this works with github, but I'm sure we can figure it out.

On Thu, Jan 25, 2018 at 6:44 AM Jacques Supcik notifications@github.com wrote:

I agree. I already have a dedicated repo for go: https://github.com/supcik/ws2811. I can complete the repo with a README file and review/update the code if you want. Then we can just link our projects.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jgarff/rpi_ws281x/issues/267#issuecomment-360469977, or mute the thread https://github.com/notifications/unsubscribe-auth/AIPem3vHyVMoNQ8025ictbTo2X2isd--ks5tOIUUgaJpZM4RszE4 .

eloots commented 6 years ago

By the way, I did a Java/Scala version for a project I’m working on: I use an 8 LED Neopixel strip on a 5 node PI cluster, one of each node, to indicate node status as seen from every node (so 5 strips in total).

The code is implicitly published as part of that project 5-node Raspberry PI - Akka Cluster. Just wanted to share this regardless of what direction you choose to take.

BTW: Thanks for creating this!!! VERY useful.

On 25 Jan 2018, at 19:53, Jeremy Garff notifications@github.com wrote:

I'm all in favor of having language specific maintainers. Great idea. I'm still however happy to maintain a central repo, with dedicated roles for various areas if that works for everyone. I'm not sure how this works with github, but I'm sure we can figure it out.

On Thu, Jan 25, 2018 at 6:44 AM Jacques Supcik notifications@github.com wrote:

I agree. I already have a dedicated repo for go: https://github.com/supcik/ws2811. I can complete the repo with a README file and review/update the code if you want. Then we can just link our projects.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jgarff/rpi_ws281x/issues/267#issuecomment-360469977, or mute the thread https://github.com/notifications/unsubscribe-auth/AIPem3vHyVMoNQ8025ictbTo2X2isd--ks5tOIUUgaJpZM4RszE4 .

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jgarff/rpi_ws281x/issues/267#issuecomment-360563304, or mute the thread https://github.com/notifications/unsubscribe-auth/ACrQ_PcONmlXe4OMG8Npe6BdoQZCzaaCks5tOM2ngaJpZM4RszE4.

supcik commented 6 years ago

Can we agree on repository names? Are rpiws281x{{language}} ok for you?

penfold42 commented 6 years ago

Good idea to split them out as separate repositories.

Another suggestion - create a new organisation and have repositories under that. rpi-ws281x / rpi_ws281x rpi-ws281x / go rpi-ws281x / python

penfold42 commented 6 years ago

@jgarff - I’ve created it already to stop a cheeky person grabbing it. If this is the approach you want to take, I’ll hand it over to you

pietrodn commented 6 years ago

I was mentioned for Golang but I don’t know Go at all :P I can give a hand on Python, though.

penfold42 commented 6 years ago

Would each binding repository reference the “C” repository as a submodule ?

This has worked well for Hyperion - we can point it at a specific commit / tag and adjust over time when new features or breaking API changes occur

siggy commented 6 years ago

@jgarff If you're interested in keeping everything in one repo, you can use https://help.github.com/articles/about-codeowners/ to designate owners of specific parts of the repo.

Gadgetoid commented 6 years ago

I'm totally on-board with @penfold42 and would love to move (and possibly rename) rpi_ws281x-python into this org if you'll have me!

I also need to redirect the submodule to the official rpi_ws281x, since I've been working from a fork that lets me be more responsive to new Pi model additions and other changes I need to expedite. I have upstreamed pretty much every feature required for Unicorn HAT/pHAT so it should be an easy switch over.

An org is how I structured the WiringPi library ports, which is a very similar case to this one: a C library with various language ports. Each port submodules the WiringPi library and binds accordingly.

See: https://github.com/wiringpi

I guess I need to:

Transfer https://github.com/pimoroni/rpi_ws281x-python into the org (and rename if necessary)
Replace the submodule with a reference to the upstream rpi_ws281x
Fork this back out to Pimoroni, so I can make and propose changes in a PR with proper due process
Start to improve the library with release notes etc and get a proper release train rolling

supcik commented 6 years ago

I worked on the golang wrapper (https://github.com/supcik/rpi_ws281x_go) for the library. Any decision on the "official" repo names?

JMurph2015 commented 6 years ago

I don't mind having federated model of maintaining bindings. Let me know where and when to put stuff. Also, we should collectively start working on public API documentation so that it's easier to keep on top of what features need to be exposed through bindings. That's something I've brushed with a couple times while making Rust bindings.

JMurph2015 commented 6 years ago

Also, I've had good luck in the past with using Git submodules (like I did with the Rust bindings I've made). It makes it easy to track another repo, and in my case I was even able to set up my build system to ensure that all submodules were initialized and downloaded before compilation. So I'm all for at least my repo using a submodule to grab the C source.

Gadgetoid commented 6 years ago

Reflecting upon this, one thing that specifically needs to happen with rpi_ws281x is versioning.

One particular benefit of this would be that we can release versioned shared library packages for Raspbian and bindings can depend upon these. See: https://github.com/jgarff/rpi_ws281x/issues/217

This also gives better visibility on fixed/outstanding bugs, new features, etc.

It's clear from the outstanding Issues and PRs that this repository needs a little more regular TLC and perhaps another eager maintainer before we try to push these ideas forward, though.

jgarff commented 6 years ago

I'm good with that. Are you volunteering? Sorry for not being able to spend as much on this project as I'd like.

On Thu, Mar 1, 2018 at 6:31 AM Philip Howard notifications@github.com wrote:

Reflecting upon this, one thing that specifically needs to happen with rpi_ws281x is versioning.

One particular benefit of this would be that we can release versioned shared library packages for Raspbian and bindings can depend upon these. See: #217 https://github.com/jgarff/rpi_ws281x/issues/217

This also gives better visibility on fixed/outstanding bugs, new features, etc.

It's clear from the outstanding Issues and PRs that this repository needs a little more regular TLC and perhaps another eager maintainer before we try to push these ideas forward, though.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jgarff/rpi_ws281x/issues/267#issuecomment-369591875, or mute the thread https://github.com/notifications/unsubscribe-auth/AIPem0q6KU2ZF6UjtEQo0FroXZOHCBasks5tZ_ghgaJpZM4RszE4 .

Gadgetoid commented 6 years ago

@jgarff it's okay! It happens. I don't blame you. I've done the same thing to repositories. Sometimes other concerns take priority and that's fine! We're not robots!

I'm conflicted about whether I should be a maintainer, since:

I have a similar problem: Time. I'm busy with a hundred or so repositories already.
I don't thoroughly understand the C code which this library is founded upon.

But on the flip side, I have a vested interest in this library and its future since Unicorn HAT and pHAT - and the code we ship to drive them - are built upon it. Aww shucks!

I suppose having a hand on the wheel would let me put the above plans into action - as we seem to have a reasonable consensus - and draw a line under the current repository state, tag it as a release and publish it to Raspbian at the very least.

JMurph2015 commented 6 years ago

Also @Gadgetoid, I was reading your comment about versioning. So, one thing that you can do if I remember correctly, is make git submodules that point to branches instead of commits so that you could make some "1.1" branch so that someone like me could track that. Then if you wanted to do a patch, you'd just start a branch for it, then make sure the relavent minor version branch gets updated.

It's a bit convoluted, but I'm not entirely sure how best to do it. There isn't exactly a C package manager.

SlySven commented 6 years ago

ℹ️ One gotcha if using git submodules on GitHub: they are not included in the archives make available from the "download the source" buttons/links should an interested party want to work from a tarball of a particular release. Then, without using git (which someone who downloads a .zip or .tar.gz copy is not likely to be using) it is impossible for the person who obtains the main project to identify which tarball-ed commit of the sub-module(s) are the ones that a particular main repository branch is linked to - if there is any sort of strong linkage to the use of particulars versions.

However, if the the main releases are given identifiable tag version markers it is possible to hand-craft additional URLs to the (suggested) submodules as other tarballs in the content on the "Releases" page of the "Code" tab here when releases are made for the main repository...

JMurph2015 commented 6 years ago

@SlySven I think the solution to that would be as you said to make tarballs ourselves in the form of "releases". I don't see a much better (in the sense of something sustainable and standardized for all of the various bindings maintainers) solution, but I'm definitely willing to be convinced. The only other way I'd thought of doing it for my Rust bindings was to use the build script to fetch a tarball itself, but then you get into how does x repo grab the source vs y repo etc. tl;dr, it would end up looking pretty different across each binding repo.

Gadgetoid commented 6 years ago

I'm very much in support of leveraging the "Releases" page to distribute proper tarballs or appropriate source/binary distributions. For my Python libraries (not specifically rpi_ws281x-python) I use the files functionality of Releases to include .deb files and Python binary .whl files and a .tar.gz of the source is also generated during the process of building those.

Well maintained releases will be key here, and depending upon how a library links against/wraps the rpi_ws281x package it may not even be necessary to include a submodule. If we're shipping an rpi_ws281x deb package and dev package then it could be managed as a dependency via apt.

JMurph2015 commented 6 years ago

@penfold42 Do you think you could add some of us to the rpi-ws281x org so we could start to test the waters on this strategy?

penfold42 commented 6 years ago

Done

Gadgetoid commented 6 years ago

I've transferred rpi_w281x-python into the new org. TODO:

Move over rpi_ws281x itself

I think only @jgarff can do this? Needs shunted over and forked back.

Nomenclature.

Should we opt for rpi_ws281x-languagename or something else?

Surgery.

We'll have to remove the Python code from this repository and update the documentation as necessary.

Python examples need updating to pep8 and migrated to an examples folder in https://github.com/rpi-ws281x/rpi_ws281x-python

I've made pains to drop the Adafruit brand-name "NeoPixel" from the Python library, although a dummy module is included for backwards compatibility.

Transfer/create other language repositories

Once we've figured out the naming scheme we need to transfer in or create empty repositories for:

go - needs to be grafted from: https://github.com/jgarff/rpi_ws281x/tree/master/golang
mono - https://github.com/jgarff/rpi_ws281x/pull/58
rust - https://github.com/jgarff/rpi_ws281x/issues/265

supcik commented 6 years ago

I have an updated repository for go here: https://github.com/supcik/rpi_ws281x_go. If you agree, (and as soon as we have a consistent naming convention) I will move it into the new organization.

JMurph2015 commented 6 years ago

Can we do either all hyphens or all underscores? Currently we have all but one permutation of that ( underscore-hyphen, hyphen-hyphen, underscore-underscore), I think ideally we would have some consistent naming scheme.

Gadgetoid commented 6 years ago

My personal preference is all hyphens. When naming my version of the Python wrapper I ended up with a mix of what I consider the upstream name: "rpi_ws281x" and then the correct way to append the language: "-python" which gives a hyphen/underscore mix. I don't really have a good argument for that particular pattern, other than it's what I did with "WiringPi-Python" "WiringPi-Ruby" etc.

I'm for renaming "rpi_ws281x" to "rpi-ws281x" when it's moved into the org (which is incidentally named "rpi-ws218x" too, if we can get some consensus? Which would make the structure:

rpi-ws281x (org)
- rpi-ws281x
- rpi-ws281x-go
- rpi-ws281x-mono
- rpi-ws281x-rust
- rpi-ws281x-python

It looks like rpi-ws281x-rust already fits this pattern.

@penfold42 could you rename "rpi_ws281x-python" to "rpi-ws281x-python" to be consistent with Rust, please? I don't have access.

supcik commented 6 years ago

It is OK for me.

penfold42 commented 6 years ago

Done - also a quick mspaint hack job of a organisation profile pic

supcik commented 6 years ago

I am building and checking my code with CircleCi. Can you please grant access to the organization to CircleCi (you should have received a mail with the request).

Thank you in advance.

penfold42 commented 6 years ago

Haven’t seen an email.

Does it need access to the whole or or just a repository ?

supcik commented 6 years ago

You granted access to CircleCi and everything is fine for me, thank you.

penfold42 commented 6 years ago

Yep - found the email in an old account

Gadgetoid commented 6 years ago

To avoid any duplication of effort we should push a deprecation warning, directing people to the new golang bindings, into the README at: https://github.com/jgarff/rpi_ws281x/tree/master/golang

Urmel11 commented 6 years ago

The mono / C# wrapper in PR #58 is not up-to-date and does not work with the latest "C" implementation. Therefore I made a new wrapper (see https://github.com/Urmel11/rpi-ws281x-csharp). Maybe we can transfer this project to the org and decline PR #58.

What do you think @Gadgetoid, @penfold42 and @jgarff?

Gadgetoid commented 6 years ago

Sheesh I just looked at the date on #58!

I'd certainly welcome your C# wrapper, although I'm not qualified to comment upon its quality or accuracy.

Yours is a good example of a wrapper that could easily depend upon a pre-packed version of this library.

penfold42 commented 6 years ago

@Urmel11 good idea, I’ll add you to the org

Urmel11 commented 6 years ago

@Gadgetoid Yes, this wrapper totally benefits from a pre-packed library version. Therefore I am really excited of issue #217.

@penfold42 Did you already added me to to org? I just wanted to transfer the ownership of my project but I get an error that I have no permissions to create a repository in the org.

penfold42 commented 6 years ago

Sorry - busy day at work.

Just done it now

Urmel11 commented 6 years ago

Thank you. I transferred the ownership now. Only a few more hours, then it is weekend ;)

Gadgetoid commented 6 years ago

Looks like we've got an empty Rust repository in the org.

@JMurph2015 do you have a game plan for getting the Rust code there?

I've closed #265 for now, but I don't mind re-opening it with a "Help Wanted" tag, since you had asked for help with hand-writing some nicer bindings.

JMurph2015 commented 6 years ago

@Gadgetoid I'm overhaulimg the Rust bindings to make them less like dirty C wrappers and more like a typical Rust crate. Call it a 0.2.x release. So I figured I'd move it then. Though I don't suppose it'd be all that difficult to move it now.

Gadgetoid commented 6 years ago

I believe we're at a point now where - pending the current PR - https://github.com/rpi-ws281x/rpi-ws281x-python is ready to take on the mantle as the way-to-do-rpi_ws281x-in-Python. I'm going to write up a deprecation warning to add to the readme here: https://github.com/jgarff/rpi_ws281x/tree/master/python

This will direct Python users to the pip package and the rpi_ws281x-python GitHub repository so we can start to migrate Python support requests, bug fixes, improvements and general progress away from this repository.

I still have some maintenance and documentation to do on the Python front, but that's of no consequence to its suitability as the new way forward.

@JMurph2015 I'm easy! It's your code and your time ;) Manage it how you see fit.

JMurph2015 commented 6 years ago

@Gadgetoid The repo is moved over, README and LICENSE also added. Still haven't gotten around to finishing the API overhaul (college is hard) but will probably be able to hit that in the early summer.

mbelling commented 6 years ago

@penfold42 do you need to give me access to https://github.com/rpi-ws281x/rpi-ws281x-java so that I can move my repository from https://github.com/mbelling/rpi-ws281x-java as mentioned in #205?

I was thinking I would do a repository transfer, but I am not sure if that works with a repo already in place or not.

penfold42 commented 6 years ago

Added you as collaborator to the java repo

mbelling commented 6 years ago

Thanks. Since your repo already exists, should I just push my repo to it instead of transferring?

penfold42 commented 6 years ago

Yep

DanielSSilva commented 6 years ago

Hello folks. So with the PowerShell being able to run on RPi, I will start a new implementation for it, probably based on the @Urmel11 Wrapper. Is it possible to be added to the organization? That way it is easier to commit directly over there, instead of committing to a repository of mine and them move it there

penfold42 commented 6 years ago

rpi-ws281x-powershell ?

jgarff / rpi_ws281x