contributing: where to start?

189 views
Skip to first unread message

Cotton Seed

unread,
Mar 23, 2014, 8:34:02 AM3/23/14
to clearsk...@googlegroups.com
I have some time and I'd like to contribute to the C++ implementation.
I don't have a clear picture of what still needs to be done, so while I
get up to speed, maybe someone could suggest a well-defined task to get
started?

A few other questions:

Is there a separate Trello board for clearskies_core? Or is
clearskies_core/TODO the place?

What's the status of the protocol? It seems like maybe there are some
changes that haven't made it into the spec yet? e.g., there is a switch
from ITC to VC/VV Trello card, but the spec still says ITC.

Best,
Cotton

Pedro Larroy

unread,
Mar 23, 2014, 2:39:00 PM3/23/14
to clearsk...@googlegroups.com
Hi Cotton

Welcome to the project.

I think there are several tasks that are a bit independent and so far we have little progress. For example, plugging the crypto library. It needs to be plugged somewhere in the IO boundary of the protocol such as ProtocolState::input. 

In  test/server.cpp it's more clear how the IO takes place, it's a test of a server and a client which are connected directly by the buffers without any socket or external IO.

Another area that needs work is plugging the uTP library and the NAT traversing paraphernalia.

Another area is the peer discovery mechanism such as broadcasting and  via tracker (as a tracker client)

Another area is the tracker server.

Another area would be any client app and help building the project in other platforms, android, linux, QT, Mac, Windows...

There are also non-programming tasks which are welcome such as design or help documenting the protocol and cleaning up the documentation, making it concise and easier to read, etc.

Currently I'm working in the manifest and update mechanism, and have some very basic things connected to libuv, but so far not a functional server yet.


Pedro.

Cotton Seed

unread,
Mar 23, 2014, 7:03:57 PM3/23/14
to clearsk...@googlegroups.com
Hi Pedro,

Great, thanks for the reply. I'll start working on the tracker
protocol, server and client code. I develop on Mac and Linux. I
already have some changes to get things building under OSX.

Cotton

Pedro Larroy:

Steven Jewel

unread,
Mar 24, 2014, 12:18:16 PM3/24/14
to clearsk...@googlegroups.com
On 03/23/2014 06:34 AM, Cotton Seed wrote:
> I have some time and I'd like to contribute to the C++ implementation.
> I don't have a clear picture of what still needs to be done, so while I
> get up to speed, maybe someone could suggest a well-defined task to get
> started?

Hi Cotton Seed! Have you had a chance to look at
https://github.com/jewel/clearskies/blob/master/license-change.md? Your
"X" is all that we're missing to finish that license change.

(Not that it matters too much, but it'd be nice for completeness sake.)

> A few other questions:
>
> Is there a separate Trello board for clearskies_core? Or is
> clearskies_core/TODO the place?

The trello board is now just my list of things that I want to do on the
protocol, most of which aren't urgent.

We probably should create github issues for TODO items, since there's no
need to reorder the items as trello can do.

> What's the status of the protocol? It seems like maybe there are some
> changes that haven't made it into the spec yet? e.g., there is a switch
> from ITC to VC/VV Trello card, but the spec still says ITC.

I believe that the protocol as documented will work, at least it runs in
my head just fine. I reorganized it addressing the issues that came up
on the list. The major difference is that divided up the protocol into
areas of concern, namely "core", "database", and "directory". Known
issues are documented within the protocol files itself, none of which
are too major.

I haven't had a chance to look at VC/VV to see which would be more
appropriate for the situation, but it should be a straightforward
conversion. I will do that this week.

Steven

Cotton Seed

unread,
Mar 24, 2014, 12:51:58 PM3/24/14
to clearsk...@googlegroups.com
Hi Steven,

I just submitted a pull request agreeing to the license change.

Thanks for the update. I'm getting a better picture of where things stand.

One last question: Am I correct in thinking Ruby proof-of-concept
doesn't reflect the latest round of protocol changes? Are you planning
to update it in parallel with the C++ implementation?

Cotton

Steven Jewel:

Steven Jewel

unread,
Mar 24, 2014, 2:06:11 PM3/24/14
to clearsk...@googlegroups.com
On 03/24/2014 10:51 AM, Cotton Seed wrote:
> One last question: Am I correct in thinking Ruby proof-of-concept
> doesn't reflect the latest round of protocol changes? Are you planning
> to update it in parallel with the C++ implementation?

You are correct. I am going to move the ruby POC to its own repository
and make sure to emphasize that it's out-of-date.

As of right now I'm not planning on updating it to the latest version of
the protocol, but most of the differences are cosmetic. (e.g. "update"
got renamed to "database.update").

I'm going to keep it around until the C++ implementation reaches feature
parity, at which point I don't believe it'll be worth having.

Steven

Dmitry Yakimenko

unread,
Mar 24, 2014, 4:20:34 PM3/24/14
to clearsk...@googlegroups.com
Guys, I found this: https://waffle.io/
It looks like Trello, but it works directly with the Github issues with two way syncing.

Dima.

On Sunday, March 23, 2014 1:34:02 PM UTC+1, Cotton Seed wrote:

Cotton Seed

unread,
Mar 25, 2014, 10:07:25 AM3/25/14
to clearsk...@googlegroups.com
I'm thinking about the tracker. I'm imagining some changes to the rest
of the code that I thought I'd run by the list before I jump in.

1. Split the Message hierarchy by message group: Core, Database,
Tracker, etc. with base visitor classes for each group. This way,
different connections can use different groups: Core, Database, Diectory
for a file syncing app; Core, Tracker for the tracker server; etc.

2. Remove global enum for message type and use type string in Message.
This makes the code more extensible, e.g., we want to add an IM message
group for a chat application.

3. Break Coder into two pieces: Codec, which converts between native C++
types and encoded values (e.g. JSON) but doesn't know about message
types (removing a O(n^2) interaction) and <group>MessageFactory, for
decoding messages from a given group.

Sound reasonable?

Cotton

Cotton Seed:

Pedro Larroy

unread,
Mar 25, 2014, 11:29:56 AM3/25/14
to clearsk...@googlegroups.com

Hi Cotton. I reply inline.

On Mar 25, 2014 3:07 PM, "Cotton Seed" <cotto...@gmail.com> wrote:
>
> I'm thinking about the tracker.  I'm imagining some changes to the rest
> of the code that I thought I'd run by the list before I jump in.
>
> 1. Split the Message hierarchy by message group: Core, Database,
> Tracker, etc.  with base visitor classes for each group.  This way,
> different connections can use different groups: Core, Database, Diectory
> for a file syncing app; Core, Tracker for the tracker server; etc.

I think this is a good idea.

>
> 2. Remove global enum for message type and use type string in Message.
> This makes the code more extensible, e.g., we want to add an IM message
> group for a chat application.

I don't know if this is a good idea. With the current implementation in C++ which is not a dynamic language, you need concrete classes for each message. Hence the enum or string doesn't make much difference. Maybe you can concrete the example a bit so we can understand what you mean better. I reason better with concrete examples :)

>
> 3. Break Coder into two pieces: Codec, which converts between native C++
> types and encoded values (e.g. JSON) but doesn't know about message
> types (removing a O(n^2) interaction) and <group>MessageFactory, for
> decoding messages from a given group.

This might be a good idea but I don't know if it's possible, maybe you want to try and propose a better improvement. Currently I made the codec swappable as you can check in the code, but I think you are right and the json codec knows about the message type. How can you convert to a C++ type if you don't know the type? This I don't know myself.

Pedro.


> --
> You received this message because you are subscribed to the Google Groups "ClearSkies Dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clearskies-de...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Cotton Seed

unread,
Mar 25, 2014, 12:27:02 PM3/25/14
to clearsk...@googlegroups.com
Reply inline.

Pedro Larroy:
> Hi Cotton. I reply inline.
>
> On Mar 25, 2014 3:07 PM, "Cotton Seed" <cotto...@gmail.com> wrote:
>>
>> I'm thinking about the tracker. I'm imagining some changes to the rest
>> of the code that I thought I'd run by the list before I jump in.
>>
>> 1. Split the Message hierarchy by message group: Core, Database,
>> Tracker, etc. with base visitor classes for each group. This way,
>> different connections can use different groups: Core, Database, Diectory
>> for a file syncing app; Core, Tracker for the tracker server; etc.
>
> I think this is a good idea.
>
>>
>> 2. Remove global enum for message type and use type string in Message.
>> This makes the code more extensible, e.g., we want to add an IM message
>> group for a chat application.
>
> I don't know if this is a good idea. With the current implementation in C++
> which is not a dynamic language, you need concrete classes for each
> message. Hence the enum or string doesn't make much difference. Maybe you
> can concrete the example a bit so we can understand what you mean better. I
> reason better with concrete examples :)

Agree; the enum doesn't make much difference. Now I think we can just
delete it. Since we're using concrete classes, the vtable should be all
we need. If we need it, we can have a

virtual std::string Message::type() const = 0;

I think the enum is only used now when creating the Message objects, but
we can just dispatch directly of the type string in the message.

My original motivation was, if an application has a protocol extension,
how does it register new message types?

>>
>> 3. Break Coder into two pieces: Codec, which converts between native C++
>> types and encoded values (e.g. JSON) but doesn't know about message
>> types (removing a O(n^2) interaction) and <group>MessageFactory, for
>> decoding messages from a given group.
>
> This might be a good idea but I don't know if it's possible, maybe you want
> to try and propose a better improvement. Currently I made the codec
> swappable as you can check in the code, but I think you are right and the
> json codec knows about the message type. How can you convert to a C++ type
> if you don't know the type? This I don't know myself.

I haven't worked out all the details, but roughly I was going to have a
JSONCodec::Value class that stores the (encoded) value, and functions like

std::string as_string(const JSONCodec:Value&);

one for each type we use and protocol extensions can add new decoders as
needed. Then you can write like

as_string(acodec["id"])

in the message constructor completely generically in terms of an
abstract Codec class. This won't work for all types, but at least it
decouples conversion of types from the construction of messages.

Alex Chamberlain

unread,
Mar 25, 2014, 12:46:32 PM3/25/14
to clearsk...@googlegroups.com
What about using the visitor pattern? Each message type would have to
have an accept method and it would dispatch to the JSON encoders, so
they knew which type they were being called from. You can get rid of
the enums that way. Ultimately, either the message types need to know
about the encoder, or the encoder needs to know about the message
types. I think the encoder knowing about the message types is more
intuitive.

Pedro Larroy

unread,
Mar 25, 2014, 6:50:10 PM3/25/14
to clearsk...@googlegroups.com
Guys, are you checking the code?


class ConstMessageVisitor                                                             
{                                                                                     
public:                                                                               
    virtual ~ConstMessageVisitor() {};                                                
    virtual void visit(const Unknown&) = 0;                                           
    virtual void visit(const InternalStart&) = 0;                                     
    virtual void visit(const Ping&) = 0;                                              
    virtual void visit(const Greeting&) = 0;                                          
    virtual void visit(const Start&) = 0;                                             
    virtual void visit(const CannotStart&) = 0;                                       
    virtual void visit(const StartTLS&) = 0;                                          
    virtual void visit(const Identity&) = 0;                                          
    virtual void visit(const Keys&) = 0;                                              
    virtual void visit(const KeysAcknowledgment&) = 0;                                
    virtual void visit(const Manifest&) = 0;                                          
    virtual void visit(const GetUpdates&) = 0;                                        
    virtual void visit(const Current&) = 0;                                           
    virtual void visit(const Get&) = 0;                                               
    virtual void visit(const FileData&) = 0;                                          
    virtual void visit(const Update&) = 0;                                            
    virtual void visit(const Move&) = 0;                                              
};      


We _are_ using the visitor pattern.

The question is that you still need to hardcode the message types and concrete classes in the visitor. It's already a bit convoluted for my taste. I don't know how this would be done if you want to extend the message types on runtime, say by loading a plugin, maybe you have some ideas how to do this without complicating the code.

I also don't want people having to read "the gang of four book" about patterns before they can contribute to the project, that's why people become scared of C++ and too much object orientation paraphernalia.

The good thing about the enum, is that is an "enum class" so it works as a type, it's easier to screw up and introduce a typo in a string. The enum class at least enforces that the variable must take one of the possible enum values and not any string.

Pedro.






Pedro Larroy Tovar   |    http://pedro.larroy.com/

Cotton Seed

unread,
Mar 27, 2014, 8:09:07 AM3/27/14
to clearsk...@googlegroups.com
I made the proposed changes. You can see them here in the feat_tracker
branch of my repository:

https://github.com/cseed/clearskies_core/compare/feat_tracker

The difference between the code and proposal:
- Added tracker messages.
- I didn't abstract the interface to jsoncons::json. This is easy but
isn't a high priority until there is at least one proposal for an
alternate encoding.
- I deleted the message types (CannotStart, StartTLS, Identity,
Current, Move) that are not included in the current spec. I commented
out server_test_01 since it uses the old messages. I also removed
InternalStart. I'm still trying to get my head around the server
control flow, but I can add it back if necessary.

Builds and tests on OSX 10.9.2 and Ubuntu 13.10. I would appreciate
some code review. If we're happy with the code, I will merge it into
master and submit a pull request. I'm going on to the daemon part.

Cotton

Cotton Seed:

Pedro Larroy

unread,
Mar 27, 2014, 12:45:49 PM3/27/14
to clearsk...@googlegroups.com
Hi Cotton

I wanted first to say thanks for your thoughts and contributions to the list and congratulations in getting your feet wet with the code.

I looked through your changes and have a question. What is the advantage of the separation of messages by deriving from three different clases? I thought you wanted to put them in different namespaces / files. What goal do you have in mind with the separation?

Doesn't having to dynamic cast to the message type break the advantage of using the visitor pattern?

I implemented the visitor pattern so we don't dynamic cast which is considered an antipattern in C++. If we are going to dynamic cast, insn't it better just to go the C way and just make a big switch to dispatch the message type altogether?

I saw that we are back with some json in the message implementation files. I think it's better to isolate the encoding a bit so we might go for a more efficient encoding once the protocol is mature if needed.

Thanks.

Pedro.



--
You received this message because you are subscribed to the Google Groups "ClearSkies Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clearskies-de...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Cotton Seed

unread,
Mar 27, 2014, 2:09:46 PM3/27/14
to clearsk...@googlegroups.com
Comments inline.

Pedro Larroy:
> Hi Cotton
>
> I wanted first to say thanks for your thoughts and contributions to the
> list and congratulations in getting your feet wet with the code.

My pleasure! I agree strongly with the goals and architecture of this
project, and I'd really like to see it succeed.

> I looked through your changes and have a question. What is the advantage of
> the separation of messages by deriving from three different clases? I
> thought you wanted to put them in different namespaces / files. What goal
> do you have in mind with the separation?
>
> Doesn't having to dynamic cast to the message type break the advantage of
> using the visitor pattern?
>
> I implemented the visitor pattern so we don't dynamic cast which is
> considered an antipattern in C++. If we are going to dynamic cast, insn't
> it better just to go the C way and just make a big switch to dispatch the
> message type altogether?

The goal was to be able to mix and match message groups in a given
protocol without having to have all message types predefined. The
tracker protocol wants core and tracker. The cs file sync protocol
wants core, database and directory (but not tracker). It sounded like
we wanted (I do!) a protocol that would support other peer-to-peer
communication services like IM that would need their own message groups.

So the ideas was to have a visitor for each message group, and inherit
the protocol from all the ones it used. ClearSkiesProtocol inherits
from those three, and TrackerProtocol (not checked in) inherits from
CoreVisitor and TrackerVisitor.

The visitor pattern is bad if the class hierarchy isn't stable. That
means a Message level visitor is a bad design if we want library clients
to be able to use new message types. That's also why I moved the
encoder/decoder functions back into the Message hierarchy.

Maybe we don't want library clients to be able to add message types?
Personally, I don't think dynamic cast is so bad if there's not another
way to get the job done. I will meditate on other ways to organize the
code.

> I saw that we are back with some json in the message implementation files.
> I think it's better to isolate the encoding a bit so we might go for a more
> efficient encoding once the protocol is mature if needed.

OK, I will abstract the interface to json so it will be easy to plug in
another encoding.

I wonder, how close will json+gzip be to a custom enconding?

Cotton Seed

unread,
Mar 28, 2014, 5:06:14 AM3/28/14
to clearsk...@googlegroups.com
I have a (hopefully) clearer articulation of what I was trying to do.
I'm basically trying to do nested visitors: visit the group, then visit
the message within the group.

However, the groups and group visitors aren't fixed, so the visitor
pattern isn't appropriate. dynamic_cast still seems like the right tool
for the job, but I didn't like doing multiple dynamic_casts. So now I
propose:

class MessageVisitor { virtual void unknown (Message&); ... }

class CoreMessageVisitor: public MessageVisitor
{
... visitor core message types ...
}

class Message
{
...
virtual void accept(MessageVisitor&) = 0;
}

class CoreMessage: public Message
{
virtual void accept(CoreMessageVisitor&) = 0;

void accept(MessageVisitor& v) override
{
if (v is CoreMessageVisitor)
accept(dynamic_cast<CoreMessageVisitor&>(v)); // second visit
else
v.unknown(*this);
}
};

Let me know if you see another way to do this. If we don't want to
support clients of the library adding message types, I'll put back the
global visitor.

Pedro Larroy

unread,
Mar 28, 2014, 8:12:52 AM3/28/14
to clearsk...@googlegroups.com
Hi Cotton

I think this is a better idea.

My suggestion is that we skip this part for now and refactor it later if we find we want it in a better way. I would suggest that you can implement the messages that you need for the tracker in the message type enum for now, just group them together and put a comment before that says they are for the tracker.

Later when we have things working we can see if it makes sense to refactor. In my opinon now we don't gain much now by prematurely overengineering this, plus if all the messages are in the same enum the intent might be even clearer, at least while we refine the protocol and the library.

Thanks a lot and regards.

Pedro.

Cotton Seed

unread,
Mar 28, 2014, 9:31:36 AM3/28/14
to clearsk...@googlegroups.com
OK, I'll change the code back to the global message visitor pattern.

I still don't see the need for the enum. Currently, the code converts
the type string to the enum, and dispatches off the enum to construct
the message object, but never uses the enum again. Isn't it better to
dispatch off the type string and create the message object directly?
With the use of virtual functions and visitor pattern, what's the point
of the enum?

Cotton Seed

unread,
Mar 28, 2014, 9:38:05 AM3/28/14
to clearsk...@googlegroups.com
> I saw that we are back with some json in the message implementation
> files. I think it's better to isolate the encoding a bit so we might
> go for a more efficient encoding once the protocol is mature if
> needed.

I'm going to again advocate for waiting on this. Scenarios:

1. If we end up just using JSON, then the encode/decode functions should
take the native JSON object as they do now.

2. If we switch to another encoding, then the encode/decode functions
should take the native representation of that encoding, so abstracting
didn't gain anything.

3. If we do end up supporting multiple encodings, then the natural
encoding abstraction might depend on the other encodings, in which
designing an encoding abstraction now is premature.

Therefore, I suggest we wait until (2) or (3) happen and revise the code
as appropriate. If you disagree, I'll go ahead and build an encoding
abstraction now.

Cotton Seed

unread,
Mar 28, 2014, 10:03:16 AM3/28/14
to clearsk...@googlegroups.com
OK, I put back the global message visitor (still without enum) and added
a default unknown member function to the message visitors so, for
example, the clearskies protocol doesn't need override the visit member
functions for the tracker messages, and so we don't need to update every
visitor when we add new (unrelated) messages.

Please advise on the encode/decode stuff.

Pedro Larroy

unread,
Mar 28, 2014, 11:07:20 AM3/28/14
to clearsk...@googlegroups.com
I think the enum for the message type is just the idiomatic way to do these kind of things in C / C++, it's typed,  saves a bit of space vs a string, and constraints the domain of the values to the ones you want, particularly using compiler warnings when you have a switch and don't handle all the values in the enum. That's the reason I think it's better to have an enum than a string. But it's not a big deal anyway.

So far I was coding the protocol behaviour like this:


So when you want to handle one message you only implement visit for the type you are interested.  I understand what you wanted to do know by dispatching an unknown message and then having different visitors for each type.  I'm not sure what's best yet so my suggestion is to park these changes to the messages and types and implement the required behavoiur for the protocol and tracker. When we have it working if we are not happy with how messages are processed then we can do a refactor in the line that you proposed. I think it's better now not to complicate this logic too much.

I think another solution for this problem that we can explore is to have an abstracted dictionary of any type such as the "jsonvalue" class from jsoncons so you can handle messages of any type depending on the data and without exposing the encoding.

But again, let's not get caught in philosophical questions about OOP, patterns and types. Let's solve the problems of synchronization, tracking, networking, consistency etc. which are the main problems that we want to solve with the product.

My 2 cents.

Pedro.



Cotton Seed

unread,
Mar 28, 2014, 12:57:23 PM3/28/14
to clearsk...@googlegroups.com
Comments inline.

Pedro Larroy:
> I think the enum for the message type is just the idiomatic way to do these
> kind of things in C / C++, it's typed, saves a bit of space vs a string,
> and constraints the domain of the values to the ones you want, particularly
> using compiler warnings when you have a switch and don't handle all the
> values in the enum. That's the reason I think it's better to have an enum
> than a string. But it's not a big deal anyway.

It's standard for C, but not C++. The virtual function table is the
standard solution for C++. Virtual functions are C++'s switch(tag).
That's why I deleted the enum: you didn't use it, and I don't see that
you plan to.

> So far I was coding the protocol behaviour like this:
>
> https://github.com/larroy/clearskies_core/blob/master/src/cs/clearskiesprotocol.cpp#L33
>
> So when you want to handle one message you only implement visit for the
> type you are interested. I understand what you wanted to do know by
> dispatching an unknown message and then having different visitors for each
> type. I'm not sure what's best yet so my suggestion is to park these
> changes to the messages and types and implement the required behavoiur for
> the protocol and tracker. When we have it working if we are not happy with
> how messages are processed then we can do a refactor in the line that you
> proposed. I think it's better now not to complicate this logic too much.

Yes, that's because you added every message type to MessageHandler. I
could add all the tracker messages (and again when we add unmanaged,
etc.) or I could just have this function on MessageHandler:

void unknown(const message::Message& msg)
{
throw ProtocolError(fs("Can't handle message type " <<
msg.type() << " on state: " << static_cast<unsigned>(m_state)));
}

and not have to worry about it again.

> I think another solution for this problem that we can explore is to have an
> abstracted dictionary of any type such as the "jsonvalue" class from
> jsoncons so you can handle messages of any type depending on the data and
> without exposing the encoding.

Yes, I started doing exactly this, but it didn't seem like a high
priority compared to, as you say, getting the essential functionality
working. That's why I proposed tabling it.

> But again, let's not get caught in philosophical questions about OOP,
> patterns and types. Let's solve the problems of synchronization, tracking,
> networking, consistency etc. which are the main problems that we want to
> solve with the product.

I don't either. I think there were some genuine organizational
questions in the code about how to support multiple protocols using
different sets of messages. I thought it necessary to address this as
part of adding tracker support. I also refactored in places where I had
to add code (e.g. encoding/decoding of tracker messages) if I thought I
could improve it. It seems you rejected those changes on the very
grounds you say you don't want to get into. I'm not trying to make
changes for no reason. My sole goal was to get the tracker put together.

I feel like you don't want me to change the existing code, but you
haven't articled how you expect me to proceed and it isn't clear (to me)
what's expected from the existing code. I spent the morning rewriting
the daemon stuff. Are you going to park those changes too? Honestly,
I'm not quite sure how to proceed with my contribution anymore.

I apologize if I'm coming off as difficult. I don't mean to be. This
is the first open source project where I've tried to jump in and make a
significant contribution. I realize there might be cultural norms I'm
unaware of.

Pedro Larroy

unread,
Mar 28, 2014, 3:22:43 PM3/28/14
to clearsk...@googlegroups.com

Hi Cotton.

When I coded the message visitor myself I was a bit critical with my own code. I also merged a pull request from Detunized that was removed in your changes.

I'm very happy that you want yo contribute and I think this is a very interesting project.

I apologize if you got the impression that your changes are not welcome its not the case at all. Your changes are most welcome. The problem is that I'm not 100% happy with the message handling code as it is now, but I'm not sure how to make it better. I'm not sure that having more dynamic casts is a good solution or not. I will have to check your patch in detail and think about it.

Whenever one makes a pull request there's the risk that it won't be merged and when you do it to contribute to a project it can suck. I agree 100%. If you have more programming power than the original project you can fork it and sail away if the contribution is so critical. For us it will be better to reach an agreement and collaborate since there's more work than programmers :-)

Again in our case your contributions and everybody else's are greatly welcome and I was trying to help answering your questions to direct the effort in a direction where you can implement a feature and design it in the way that you judge that makes most sense and I can merge it if its mostly OK.

If you refactor the message handling which is a central piece in a way that is not better I won't be able to merge, that's a risk that you have to accept and can minimize by asking first as you did on the list. So there's no etiquette that you breached and I was more than happy to answer your questions. I'm also very happy that you want to contribute.

But you shouldn't get discouraged so easily because your changes are questioned or even not merged. You might think that you waste your time but its not like that. You got familiar with the code and you got feedback on your solution and this dialog is what make professionals grow. Please don't give up or get discouraged by my opinions. We were just analyzing the code, anything beyond that is your personal interpretation.

Having said that. I had a very busy week and only looked at your changes briefly on the web interface. If you feel your changes really improve that code make a pull request and I will check it carefully and we can discuss it here to see what's best. We also don't want to make the code more difficult to understand for other people that might want to contribute for a marginal benefit.

I just wanted to tell you that anything you do on the tracker is easier to merge than refactoring the message handling which BTW has merges from 3 different people . They might also object if we remove their contribution without analysis. I dont like that code 100% and if i change it i want to make sure it makes it better not worse.

I hope I answered your concerns. Please dont get discouraged by what a stranger on the internet says. I think this is an interesting project and we can have a lot of fun coding and make a great product together.

Again if you are confident that your change is good, make the pull and I promise to study it in detail. Just dont be upset if I try to analyze it ;-)

Pedro.

Cotton Seed

unread,
Mar 29, 2014, 11:22:49 AM3/29/14
to clearsk...@googlegroups.com
Hi Pedro,

Thanks for the thoughtful reply.

I see a potential miscommunication. I had already removed the dynamic
cast and put back a single message visitor pattern with all the message
types before your email that mentioned parking my changes. I had
already emailed the list, but I see now my email was not clear. Hence
the confusion: I had updated my changes based on your feedback but you
didn't seem to want them nor did you say what else was wrong.

You can see the current proposed changes here:

https://github.com/cseed/clearskies_core/compare/feat_tracker

I'm happy (even eager) to fight things out as long as I can see how to
make forward progress.

Also, you wrote:
> I also merged a pull request from Detunized that was removed in your
> changes.

You mean I was behind the mainline? Of course I have to fix that before
I submit a pull request. Anyway, I just updated my branch so it should
be good for the moment.

Anyway, please have a look at the proposed changes when you have the
time. If you're still not happy, I will updated as needed or politely
disagree. :-) In the meantime, I'm going to continue hacking on the
tracker server.

Cotton

Pedro Larroy:

Dmitry Yakimenko

unread,
Mar 30, 2014, 7:33:52 AM3/30/14
to clearsk...@googlegroups.com
I was wandering if the tracker should be in the main codebase at all. It's not core. It's not required to exist for two peers that know each other to exchange files.

How much code would the tracker be sharing with the core/client? Would it be better to write it in something more wrist friendly, like Python or Scala for instance? Tracker would be running on the server and being a native app is not a requirement IMO.

What do you think?

Pedro Larroy

unread,
Mar 30, 2014, 8:02:50 AM3/30/14
to clearsk...@googlegroups.com

Where were you wandering?
I agree with what you wrote. If I remember correctly is just json messages without payload etc. Should be easy to make in any language.

--

Cotton Seed

unread,
Mar 30, 2014, 10:30:01 AM3/30/14
to clearsk...@googlegroups.com
I finished a first cut of the tracker. The tracker protocol uses the
clearskies wire protocol, so the both client and server use the message,
protocol and daemon library components.

I assume we will include the tracker client in the clearskies client. I
don't find the C++ particularly onerous (esp. with nice abstractions for
messaging, etc.), so I don't really see what's gained by reimplementing
it in another language. All told, the server is about raw 280 lines.
For now I put it in src/tracker.

Cotton

Dmitry Yakimenko:

Cotton Seed

unread,
Mar 30, 2014, 10:33:46 AM3/30/14
to clearsk...@googlegroups.com
From the spec:
> Communication with the tracker is done with clearskies messages,
> which are encoded using the "wire protocol", as was explained
> earlier.

The control protocol is just json.

Cotton

Pedro Larroy:

Pedro Larroy

unread,
Mar 30, 2014, 8:44:48 PM3/30/14
to clearsk...@googlegroups.com
Cool! I will have a  look as soon as I have time. I'm on holidays for a couple of weeks now.

Regards.


--
You received this message because you are subscribed to the Google Groups "ClearSkies Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clearskies-de...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--

Steven Jewel

unread,
Mar 30, 2014, 11:18:00 PM3/30/14
to clearsk...@googlegroups.com
On 03/30/2014 06:44 PM, Pedro Larroy wrote:
> Cool! I will have a look as soon as I have time. I'm on holidays for a
> couple of weeks now.

Have a good holiday! Hopefully we have mountains of pull requests ready
for you when you get back. :)

Steven

Pedro Larroy

unread,
Mar 31, 2014, 7:28:45 PM3/31/14
to clearsk...@googlegroups.com, clear...@stevenjewel.com
Thanks! I guess I will be able to connect periodically from hotels.

Pedro Larroy

unread,
Apr 28, 2014, 3:09:40 PM4/28/14
to clearsk...@googlegroups.com
Hi

After speaking to detunized I devised a solution for the coupling problems with the message class with the protocol. I will refactor accordingly so we can have different message coders for core, control and extensions. I think this was the problem that Cotton identified in the first place.

Regards.

pav...@gmail.com

unread,
Jun 25, 2014, 3:42:01 PM6/25/14
to clearsk...@googlegroups.com
Hello,

Is the project still alive? I am currently looking for a project that I can contribute to :)

BR,
Pawel

Pedro Larroy

unread,
Jun 26, 2014, 6:30:02 PM6/26/14
to clearsk...@googlegroups.com

Hi Pawel

What would you like to help with?

Pedro

pav...@gmail.com

unread,
Jun 27, 2014, 6:58:05 AM6/27/14
to clearsk...@googlegroups.com
Me and my two friends from work are C++ developers and we're looking for a welcoming Open Source project :)

BR,
Pawel.

pav...@gmail.com

unread,
Jun 27, 2014, 1:01:27 PM6/27/14
to clearsk...@googlegroups.com, pav...@gmail.com
So answering directly your question: coding!

Pawel

Pedro Larroy

unread,
Jun 27, 2014, 6:04:27 PM6/27/14
to clearsk...@googlegroups.com

Hi Paweł

I can give you more detail on request. We have an effort for a C++ engine for the protocol. It needs a bit of work to get the protocol going. (Should be protocol.CPP) I'm rethinking some approaches and I think we should send the files on chunks with checksums as they do in syncthing. Im a bit stuck deciding what to inject messages during a file transfer, how to queue update notifications etc. Mostly timing and protocol....

Then there's also the crypto layer which needs to be done.

What is the most interesting part for you?

Pedro.

On Jun 27, 2014 7:09 PM, <pav...@gmail.com> wrote:
So answering directly your question: coding!

Pawel

Ranky

unread,
Aug 14, 2014, 12:28:41 PM8/14/14
to clearsk...@googlegroups.com
Hi Pedro,

I can contribute by coding given some initial guidance. Please advice.

Thanks,
Ranky

Pedro Larroy

unread,
Aug 15, 2014, 5:17:49 AM8/15/14
to clearsk...@googlegroups.com
Hi Ranky

First of all we should decide if we want to be compatible with syncthing or not. I think there would be a great value to have compatibility with syncthing given that the protocol works and is proven, and some people would like to have a C++ client.

If we don't want to follow the syncthing protocol (I think we should consider). I would change the message protocol to use a more efficient encoding such as msgpack.

For syncrhonizing files the basic stuff is there already at 80-90%  so more work would be needed in the protocol side.

Pedro


--
You received this message because you are subscribed to the Google Groups "ClearSkies Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clearskies-de...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Krishna Prasad

unread,
Aug 19, 2014, 12:28:48 PM8/19/14
to clearsk...@googlegroups.com
+1 for syncthing.

Ranky
Reply all
Reply to author
Forward
0 new messages