durin42 added a comment.
In https://phab.mercurial-scm.org/D1551#27325, @smf wrote: > durin42 (Augie Fackler) <phabrica...@mercurial-scm.org> writes: > > > durin42 added a comment. > > > > I'm landing this series as-is, even though there are some good ideas for how we could improve the storage layer. My reasoning is this: we've had this as a semi-popular extension for literally years, it's functionality that we know is useful, and it'd be better to land something good rather than never land it in the name of wishing for something better. We've got almost two full months left in the cycle before this even ships as an experimental feature, so if anyone wants to try and iterate aggressively on the format I'd be delighted to talk over goals and review patches (but I'm far too busy to do any coding work, alas.) > > I hadn't finished writing my review of this patch, so that's awesome. I > have many concerns for this as-is. We need to have some kind of > measuring stick for quality in core instead of saying, "perfect is the > enemy of good," and letting everything through. Sean, the last state I had from you[0] was requiring three-step refactor that involved not-small work on at least blackbox and journal, as well as on remotenames. I don't think that's realistic (and so kills remotenames as something we can ship any fraction of in core), and it's been weeks since we had any action on that last round of patches. As far as I can tell, everyone that was part of that discussion agrees in broad strokes with your notions, but nobody is willing to do the work, certainly not as a blocker for landing this functionality in core. 0: https://phab.mercurial-scm.org/D1358#23866 > I'm not asking for perfect. I'm asking for this to be reworked to > something consistent for core. We haven't had a file format so external > pushed to core so quickly before. Histedit, journal, schemes, and even > mq have their own file formats which have been ironed out in hgext/. I'm confused. histedit landed in core in a single go, with no review on the file format (it was pickle! the worst possible choice!). The file format we're looking at here for remotenames is substantially similar to the one that's been in remotebranches for nearly a decade, so forgive me if I'm dubious that a file format refactor should be a blocker here. > For this patch as-is, it should at the very least be named > logexchange.py or something similar. "remotenames" is weird name to have > in core and already conflicts with the name of the extension it comes > from. Per irc, Pulkit already has a patch ready to fix that. > I plan to back this out because there are so many things that need > to be worked out before this lands. Do not back this out without getting someone else to agree with you first. As of now I see a new complaint in this message from you (please rename this) which is fine, and is something we can roll forward on. > What happened to our quality for core? Patience. If I'd known about the naming concern I would have waited to land this, but I never saw that complaint in the almost a month this patch (or its precursor stack related to https://phab.mercurial-scm.org/D1358) have been visible. I can't read minds, and it's consistent with past decisions to bring things in without name changes (we did that on histedit!), but I see why we should behave differently here (since we're only bringing parts of remotenames to core.) Does that help clarify things? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1551 To: pulkit, #hg-reviewers, durin42 Cc: grim, smf, durin42, dlax, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
