martinvonz added inline comments. INLINE COMMENTS
> httppeer.py:95 > + raise ValueError( > + 'fileprepender only supports file objects that ' > + 'have a length but this one does not:', type(f), f) I'm guessing 'fileprepender' here and a few placed below is the old name for '_multifile' (so please update) > httppeer.py:96 > + 'fileprepender only supports file objects that ' > + 'have a length but this one does not:', type(f), f) > + self._fileobjs = fileobjs It doesn't matter much since it's just a programming error if it happens, but how will these arguments to ValueError be rendered? > httppeer.py:105 > + def read(self, amt=None): > + if not amt: > + return ''.join(f.read() for f in self._fileobjs) i read that negative amount means the same thing (read all). do we care to follow that contract here? > httppeer.py:111 > + got = len(parts[-1]) > + if got < amt: > + self._index += 1 nit: i think this can be "if got <= amt" to avoid an unnecessary 0-length read the next time _multifile.read() (and it does look like that will be the normal case, that the reader will read exactly the size of the first "file" first) > httppeer.py:125 > + 'could be fixed if you need it') > + for f in self._fileobjs: > + f.seek(0) also need to set self._index=0, no? > httppeer.py:193 > strargs = urlreq.urlencode(sorted(args.items())) > if strargs: > if not data: Nit: to reduce indentation, it looks like you can change the condition above to "if postargsok and args" instead (because bool(strargs) == bool(args) AFAICT) REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D231 To: durin42, #hg-reviewers Cc: martinvonz, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
