On Wed, 15 Feb 2017 13:29:30 +0000, Gábor STEFANIK wrote: > > >> -def updatetotally(ui, repo, checkout, brev, clean=False, check=False): > > >> +def updatetotally(ui, repo, checkout, brev, clean=False, > > >> + updatecheck='linear'): > > > > > > Please make this updatecheck=None, and set it to 'linear' inside > > > merge.update() explicitly if None was passed to that function. > > > > Will do. > > > > > > > > This way, in patch 6, you can red the config option inside > > > merge.update(), and let other callers (primarily extensions) also > > automatically follow the config option. > > > > I don't think it's a good idea for the low-level merge.update() to read the > > config. That function is called e.g. by rebase and graft and they should not > > have their behavior changed by this config.
I tend to agree with Martin, low-level functions should have less dependency on config. Instead, maybe we could add a function that builds options from ui (like patch.diffopts), but I don't know if that's appropriate here. > I forgot to add, I'm particularly concerned about the "guestrepo" extension, > which we use extensively at NNG. IIRC in the "grupdate" command, it doesn't > use commands.update, and so changing the config option causes "update" > and "grupdate" to behave differently. > > Also, care must be taken about subrepositories. Not sure if our subrepo update > code calls command.update - if not, we may even end up with a situation > where "hg update" in a repository with subrepos with ui.updatecheck="merge" > will do nonlinear updates with a dirty main repo, but not with a dirty > subrepo. IIRC, updating including subrepos isn't a simple cascading operation. Perhaps users would be asked how to process changes in subrepos? > In fact, if we introduce a config option, but only use it in > commands.update(), > we should make the lower functions error out or at least warn if called > without > explicit updatecheck, and without other options (e.g. branchmerge) that > override > linearity checks. This way, we at least catch extensions that don't follow the > config option, but try to rely on merge.update()'s linearity checks. Good point. _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel