D2593: state: add logic to parse the state file in old way if cbor fails

pulkit (Pulkit Goyal) phabricator at mercurial-scm.org
Mon Mar 26 11:08:38 EDT 2018


pulkit added inline comments.

INLINE COMMENTS

> martinvonz wrote in state.py:84
> How about not requiring it to be a dict? I imagine practically all callers will want to pass a dict, but why does this class have to enforce it? I think the  API would be simpler if it was an opaque object. In the simple case of graft, the state is simply a list of nodes. However, for more complex states, we could have nested structures. I don't see why cmdstate should be involved in lookups into the top-level structure. The difference is subtle, but here's an example:
> 
>   # With dict-aware cmdstate
>   cmdstate = ...
>   cmdstate.load()
>   version = cmdstate['version']
>   for car in cmdstate['cars']:
>      for wheel in car['wheels']:
>          # whatever
>   
>   # With agnostic cmdstate
>   cmdstate = ...
>   parking = cmdstate.load()
>   version = parking['version']
>   for car in parking['cars']:
>      for wheel in car['wheels']:
>          # whatever

I don't have a strong reason. I just found this an easy way. I was going to add __setitem__() too so that we can store more new values or change existing values.

For graft case, it won't be just a list of nodes, we are going to store info about `--no-commit` flag in graftstate. Moreover we need `graft --abort` too which needs to store more things.

The one benefit is that we can pass the state object everywhere, lookup for values and update values on fly.

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers
Cc: martinvonz, indygreg, durin42, mercurial-devel


More information about the Mercurial-devel mailing list