D1144: directaccess: add support to export and tests to demonstrate things

durham (Durham Goode) phabricator at mercurial-scm.org
Fri Oct 27 13:04:47 EDT 2017


durham added a comment.


  @lothiraldan
  Are pinned revs deleted when the cache is rebuilt?  I would kinda not expect them to be.  If they are deleted, then yea moving these exceptions to another attribute that is persisted across cache clears makes sense.
  
  For your concern about changing from a command-level option to a scope and revrange level option, I have a couple comments:
  
  - If we went the current command-level flag, the examples your talking about where it's initially ambiguous if the command is actually reading or writing the hidden commit (phase, fold --reuse-msg, etc) would default to warn mode and the only negative effect would be the user gets an extra warning message telling them a hash they passed is hidden. Probably not a common occurrence, and not a huge issue.  In the future we could add more logic within the command to suppress that warning when doing the read only paths, but for the short term this let's us get directaccess out the door without auditing and updating every commands logic.
  - As for adding exception context manager type things, I think one of the issues with our current visibility code is that it requires the person writing a command to be too aware that visibility exists.  If we start requiring commands to use this new context appropriately, and to pass the right argument to revrange at the right time, I think we just introduce more risk of commands being written incorrectly and having more complexity at the business logic layer.  If we can avoid all of that at the cost of printing warnings to the user slightly more often, I think that's a good trade off.

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers, lothiraldan
Cc: durham, dlax, lothiraldan, mercurial-devel


More information about the Mercurial-devel mailing list