quark added a comment.

  The main feature and implementation looks good to me. My only question is 
whether `:minversion` is needed or not.

INLINE COMMENTS

> revset.py:2137-2157
> +    ourversion = util.versiontuple(n=2)
> +
> +    for key, value in options.iteritems():
> +        if ':' in key:
> +            continue
> +
> +        # Revsets in repo may require features not available to old clients.

Is this version check necessary practically? For example, without the 
minversion check, defining `P=X` where `X` is only available in a new version. 
Old client will get `X undefined`. With the minversion check, old client will 
get `P undefined`. I think `X undefined` or `P undefined` is not that much 
different.

revset could also be defined by extensions. People can also polyfill old 
mercurial so they have new revsets. It seems to be a lot of work if we also 
detect extension enabled and their versions.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D98

EMAIL PREFERENCES
  https://phab.mercurial-scm.org/settings/panel/emailpreferences/

To: indygreg, #hg-reviewers
Cc: quark, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to