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's PR on adding history to contract archives + fixes #713

Open
wants to merge 3 commits into
base: poc-slc
Choose a base branch
from

Conversation

jeromesimeon
Copy link
Member

@jeromesimeon jeromesimeon commented Jan 24, 2022

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. It has 3 commits on top of that branch instead of 42. The code was not rebased but instead is in a single new commit here: dca4657

  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: 759fe68
  2. Cicero-server tests fail due to a missing directory. This has been fixed by adding that directory back in the following commit: b21ce11

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 jeromesimeon changed the title Sanket slc history Sanket's PR on adding history to contract archives + fixes Jan 24, 2022
@jeromesimeon jeromesimeon marked this pull request as ready for review February 23, 2022 02:26
@jeromesimeon
Copy link
Member Author

Turning this into a proper PR. This includes all the work from #709 which is now closed.

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.

1 participant