Change in vdsm[master]: stomp: parsing buffer refactoring

fromani at redhat.com fromani at redhat.com
Wed Mar 18 09:39:57 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: stomp: parsing buffer refactoring
......................................................................


Patch Set 4:

(1 comment)

I trust you infra devs, and I can imagine tracking down rationale for this patch may be not-trivial, but we still lack on documentation.

https://gerrit.ovirt.org/#/c/38666/4//COMMIT_MSG
Commit Message:

Line 7: stomp: parsing buffer refactoring
Line 8: 
Line 9: Refactored how we manage buffer and parser state in Parser class. Fixing
Line 10: spelling in Subscription class name and extending default buffer size in
Line 11: AsyncDispatcher.
This is an improvement, but we still lack rationale for these changes, which should be the backbone of the commit message.

To make things easier, you may want to split this patch like:
- spelling mistake fix (no-brainer to review and verify)
- increase of buffer size to 4096 (maybe move that to a constant?), trivial to review, hopefully easier to provide rationale
- core refactoring of the buffer, move from cStringIO to string. I still don't get why we need so. Well, I can imagine, but that's it.
Line 12: 
Line 13: Change-Id: I1b835e0d8a5ca20e67f0562955255ba1ad6ae9a1
Line 14: Signed-off-by: pkliczewski <piotr.kliczewski at gmail.com>


-- 
To view, visit https://gerrit.ovirt.org/38666
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1b835e0d8a5ca20e67f0562955255ba1ad6ae9a1
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list