-
Notifications
You must be signed in to change notification settings - Fork 12
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
Namespace might conflict with other packages #27
Comments
makes sense, so let's go through the shopping list:
C++ fallout (1) - dereferenced names: `grep -r namespace .|grep 'pb;' ./machinetalk/support/encdec.cc:using namespace pb; easy, I can take that - should go for fully qualified names in C++ from now on C++ fallout (2) - fully qualified names will take that, single emacs mass reedit nanopb C fallout - not much, can take that Since the wireformat doesnt change, that change is local to the machinekit repo @Strahlex - what would be involved on your side to make QtQuick VCP build from a mutated machinetalk-protobuf branch? will your source be backwards compatible or is that forward only? I think step 1 would be to temporarily introduce a branch in this repo which has the new namespace conventions, then merge that into machinekit as a subtree - that enables fixing the mk repo independently Alex can have a got at it, until then keeps using the current branch depending on results we might have to do a synced upgrade, or a independent one |
@mhaberler The code should be pretty much compatible since I do not use the Makefile to generate my protobuf classes anyway. I think we can go forward on this topic as it simplifies using Machinetalk on non-Machinekit computers significantly: |
Has been merged. |
The namespace
pb
might conflict with other packages as it is too generic. Protobuf uses a namespace that is more descriptive and unique (google.protobuf
). When we want to install the protobuf files into the right directories, we need to make sure it doesn't overlap with other packages.I do not know what the consequences are for nanopb, but I think this is the right place to discuss that.
I think this needs to be done before we roll out separate packages as described in #24, #25 and #26.
The text was updated successfully, but these errors were encountered: