Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanket slc history #712

Closed
wants to merge 26 commits into from
Closed

Sanket slc history #712

wants to merge 26 commits into from

Conversation

jeromesimeon
Copy link
Member

Description

This is a draft PR to replace #709 which addresses some of the main issues with the original PR:

There are three major blocking issues with this PR:

  1. Not properly based on the poc-slc branch

This PR is a proper extension of the poc-slc branch. The code was not rebased but instead is in a single new commit here:

  1. deletes the whole cicero-transform package, instead importing a fixed version (why?!)

This PR properly depends on the cicero-transform package which is still present instead of deleted

  1. tests are not passing (not sure why they aren't triggered on this PR). Below is a log

Two different issues with testing were fixed (with minimal knowledge of the intent):

  1. some tests fails when the contract archive does not have history. this has been fixed by making history optional in the following commit:
  2. Cicero-server tests fail due to a missing directory. This has been fixed by adding that directory back in the following commit:

As discussed during the last Tech WG call, I'm looking into recreating this PR properly.

Changes

  • Changes from feat(*): state management of contract instances #709 for storing contract state in .slc archives
  • Fixes issues with archives with no history
  • Fixes testing issues with cicero-server changes needing a missing directory
  • Fixes dependency issues with @accordproject/cicero-transform

Flags

  • Could not do a proper rebase due to a dirty git history and the large number of changes, this version loses authorship and individual commits from @sanketshevkar
  • Because of the testing issues and "hard reset" of the code, I have a somewhat lower confidence that this is a proper reflection of the original code and intent from its author. I will leave this leave this as a draft PR until I can get a review from @sanketshevkar and @martinhalford and confirmation that this is right

jeromesimeon and others added 26 commits October 28, 2021 05:55
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 96.905% when pulling b21ce11 on sanket-slc-history into 719594c on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants