pulkit added inline comments.
INLINE COMMENTS
> martinvonz wrote in narrowcommands.py:293-295
> As I said on IRC, I think we (Google) would appreciate it if this could
> remain here for a while to give us more time to migrate our custom server to
> the new wire protocol command. I was thinking we could just fall back to the
> getbundle command if narrow_widen isn't supported. That probably means
> changing the capability to "exp-narrow-2" now and call getbundle if
> "exp-narrow-1" is supported but "exp-narrow-2" isn't. That shouldn't be very
> difficult, but I don't think we've done kind of thing before.
>
> Actually, you can ignore all that because our server has the ellipses
> capability, so it will not get here anyway :) We should instead think about
> such a migration path when we add support for ellipses to narrow_widen.
We can have a 'exp-ellipses-2' which will tell whether the server supports
ellipses widening using narrow_widen() wireprotocol command or not. I think
that should help in the meantime. Also will a week, or 10-15 days be enough for
you? I think it will be better if can prevent releasing this compatibility
because exp-ellipses-1 was introduced in this cycle only.
> martinvonz wrote in narrowcommands.py:302
> Should we call this something like "heads" instead? It's the set of common
> heads between the server and client and "common" makes sense together with
> "heads" in the getbundle call, but it doesn't seem to make as much sense here.
Made this commonheads which makes more sense. Thanks for the suggestion!
> martinvonz wrote in narrowcommands.py:307-310
> It looks like this doesn't need to be in in the commandexecutor block and the
> transaction probably doesn't need to span the wire protocol request. IOW, I
> think you can drop `repo.transaction()`from line 293 and instead put that
> togetgher this with this with-block.
Did that, thanks!
> martinvonz wrote in narrowwirepeer.py:49
> Should we just put this in core from the beginning?
I implemented the peer initially in core only, but while implementing server
side, I realized it rely on logic in narrowbundle2.py which also needs to be
moved to core. Then I decided to implement it cleanly in the extension and then
move it to core.
> martinvonz wrote in narrowwirepeer.py:79-82
> I suppose we want the default to be False? Isn't `ellipses =
> (args.get('ellipses') == '1')` enough? Or is the idea to be flexible with
> what values we accept? Is that what other wire protocol commands do?
I am not sure, I just copied from getbundle() handling:
https://www.mercurial-scm.org/repo/hg/file/1a4c1a3cc3f5/mercurial/wireprotov1server.py#l404
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D4813
To: pulkit, durin42, #hg-reviewers, martinvonz
Cc: indygreg, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
