[PATCH 1 of 9 changegroup-apis] exchange: use early return

Kevin Bullock kbullock+mercurial at ringworld.org
Thu Aug 4 09:58:17 EDT 2016


> On Aug 3, 2016, at 22:53, Gregory Szorc <gregory.szorc at gmail.com> wrote:
> 
>> On Wed, Aug 3, 2016 at 1:56 AM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
>>> On 08/02/2016 04:50 AM, Gregory Szorc wrote:
>>> 
>>> I think the end state of this series involves a "changegroup emitter"
>>> being fed into some kind of "bundler," preferably as a generator. Using
>>> the cg1unpacker for this purpose is kind of silly, as it necessitates a
>>> util.chunkbuffer. The good news is the "changegroupemitter" as it stands
>>> can easily be represented as both a generator of chunks or a file
>>> handle. So by introducing this object and requiring callers to use it,
>>> we can start transitioning callers to not use util.chunkbuffer without
>>> having to change APIs on changegroup.py. It's much more flexible.
>>> 
>> This paragraph highlight what seems to be the most important thing to
>> address in this discussion. You seems to try to avoid touching the
>> existing changegroup API as much as possible and I do not understand
>> why. My general feeling is that it should be possible to upgrade the
>> existing class to fit your needs, but your seems to think otherwise. I'm
>> curious to find out why as you usually have pretty good reason for doing
>> things?
>> 
> I avoided changing the cg<N>packer and cg<N>unpacker classes because I didn't want to write a 40 part series :)
> 
> The cg<N>unpacker class in particular is a bit gnarly because it performs multiple tasks:
> 
> 1) Exposes a file API from a possibly compressed input file object (read, seek, tell, close)
> 2) Defines the composition of the changegroup framing protocol
> 3) Converts a file object to a generator of changegroup chunks
> 4) Applies a changegroup to a repo
> 
> This class is doing too much and needs to be split. To be honest, I'm not sure how. I do know refactoring it will be a long series :)

Sounds like we should come up with a strategy for this at the sprint, if it's not resolved by then. I've added it to the wiki page.

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock



More information about the Mercurial-devel mailing list