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