Conversation
aszlig
left a comment
aszlig
left a comment
There was a problem hiding this comment.
Thanks a lot and nice work :-)
As commented already, I've been very strict in adhering to PEP-8, including the line length limit of 79 characters. Adding code that doesn't adhere to that would make the code base inconsistent.
Other than that and the mentioned comments, it looks good to me.
| class SubAccount(object): | ||
| def __init__(self, conn, box_id_, result): | ||
| self.conn = conn | ||
| self.box_id_ = box_id_ |
There was a problem hiding this comment.
Why did you call this box_id_ instead of just box_id?
There was a problem hiding this comment.
I think it was to keep consistent with the StorageBox object, which uses self.id_. I could change id to box_id
Co-authored-by: aszlig <aszlig@redmoonstudios.org>
| infolist = [u"{0}: {1}".format(key, val) | ||
| for key, val in info.items()] | ||
|
|
||
| self.putline(u"{0} ({1})".format(storagebox.login, u", ".join(infolist))) |
There was a problem hiding this comment.
Just was about to merge this pull request and did a small final review and now wondering why you use Unicode literals here: Do you still use Python 2.x and if yes, what's the reason? I'm asking because I wanted to get rid of Python 2 support, but I might re-evaluate if there are still Python 2 users out there.
There was a problem hiding this comment.
No, I just use python3. Not sure why I used them, maybe because they were used somewhere else?
|
It's been ready for a year, @aszlig Merge the review already So I don't have to use your robots api dirrectly and it'd be a pain |
3 participants
Based on https://robot.your-server.de/doc/webservice/de.html#storage-box