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

Pierre-Yves David pierre-yves.david at ens-lyon.org
Mon Aug 1 21:44:23 EDT 2016



On 08/01/2016 08:20 PM, Gregory Szorc wrote:
> This is the first half of the series. The second half can be found at
> https://hg.mozilla.org/users/gszorc_mozilla.com/hg/rev/26eeb1e66e88 if
> someone wants to review the whole thing.

I've made a first review pass on this series. As much as I like the idea
of cleaning this up, the proposed implementation makes me uncomfortable.
The main points that troubles me are:

* the extra new class is a bit of a mix of (1) outgoing object (2) cg1
bundler object. This makes a the boundary of these object more fuzzy and
I not convinced yet your cleanup could not happen with the existing objects,

* many @class method usage while our coding style usually avoid them
along other "class as namespace" usage (as per Matt taste),

* the
superlongnameforeverything.thatcombinewithclassnamspacing.foroverninethousand
charline we could probably me more compact using a abbreviation for
common parts as we do elsewhere in the code.

Let me do a full scan of the second part of the series tomorrow and then
I either understand the purity of your way, or find some super precise
feed back, or we might discuss it over voice.

> Word of warning: if you pull that repo, you'll get tons of obsolescence
> markers. You may want to disable obsolescence when pulling.

These are legit markers that do not exist in the repository because we
use email and we somewhat managed to not pull from you in the past. We
should be okay pushing them to the main server (the marker/changeset
ratio is a bit high but fine).

Cheers,

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list