accordproject / template-archive

Smart Legal Contracts & Templating System
https://accordproject.org/projects/cicero/
Apache License 2.0
280 stars 119 forks source link

feat(*): state management of contract instances #709

Closed sanketshevkar closed 2 years ago

sanketshevkar commented 2 years ago

Changes

Author Checklist

coveralls commented 2 years ago

Coverage Status

Coverage decreased (-1.08%) to 96.198% when pulling b5fae6fff8aa250dc225bf3b1a428dec0fa3a54a on sanketshevkar:sanketshevkar/state-management-slc into 719594c11be69db165fbdb6a95bc917d991da49c on accordproject:master.

jeromesimeon commented 2 years ago

this is great work but this shouldn't be merged into master at this stage. I would suggest instead to merge into the slc branch where we can address the general question around "what is the slc format" along with all the questions that Dan already asked.

Another option is to: make a push for a proper "slc" format in the slc branch and first merge that, then integrate the additional work which is in this PR. (instead of trying to merge both at the same time).

Also, it would be useful to know how this PR relates to #698 (is it just an extension, a rewrite, etc). The merge conflicts makes me wonder if it is based on a slightly outdated version of #698.

jeromesimeon commented 2 years ago

@martinhalford my suggestions to get Sanket's work available for further fixes in a short amount of time:

  1. push the branch "as is" to the repo. I believe Sanket should have the permissions to do that?
  2. have someone rebase Sanket's branch on top of poc-slc
  3. do additional PRs as needed to address Dan's comments

Let me know what you think. The benefit of this is that after 1. anyone can contribute to the work.

jeromesimeon commented 2 years ago

I believe all of those commits are already in the poc-slc branch which seems to indicate that this PR hasn't been properly based on the latest poc-slc.

Screen Shot 2021-12-15 at 2 28 12 PM
martinhalford commented 2 years ago

Hi @jeromesimeon, Ok. I'm in agreement if you and @dselman are happy to accept the PR "as is".

(Cc: @sanketshevkar - see above)

jeromesimeon commented 2 years ago

@martinhalford @sanketshevkar A new PR has been open which replaces this PR at #713 . It would need review to make sure this properly reflect the original work.

jeromesimeon commented 2 years ago

This work is now included in #713