search

Raven24 / diaspora-federation

MIT License
18 stars 5 forks source link

refactor entity and xml_payload #3

Closed fabianrbz closed 11 years ago

fabianrbz commented 11 years ago

First of all, this is an awesome work! While reviewing this some rafactors came up, I still have to take a look at some parts of the code, I'm not that familiar with Diaspora's federation, but I'm still learning!

I notice that there are some #TODOs in the code, I can work on them if you like, but probably I'm goning to need some help to understand what is needed to be changed.

coveralls commented 11 years ago

Coverage Status

Coverage increased (+0.06%) when pulling 2b396f97e1253c7a0789ecc535f366b64dddbc53 on fabianrbz:refactor into a7071001118e53373c2a5f97e363cf30521032be on Raven24:master.

Raven24 commented 11 years ago

thank you very much for reviewing! I will look at this in more detail tomorrow, from a first glance it seems really good. I sometimes fall back into non-ruby patterns, so any feedback regarding code-style is very welcome.

the problem with the #TODOs is, that they are mostly referencing the problems in the current federation implementation, like fields that don't make sense or stuff that is missing. There should be some discussion about how to handle those issues best, also keeping in mind backward-compatibility...

fabianrbz commented 11 years ago

@Raven24 I see, I'll try to take a look at the #TODO's when I have more time

Raven24 commented 11 years ago

thank you! ;)