Skip to content
This repository has been archived by the owner on Jun 21, 2020. It is now read-only.

BLD: adjusting to core & ppal msgs #123

Closed
wants to merge 12 commits into from
Closed

BLD: adjusting to core & ppal msgs #123

wants to merge 12 commits into from

Conversation

lacabra
Copy link
Contributor

@lacabra lacabra commented Apr 3, 2019

  • Adds scheme for GetPTTRequest that was empty
  • Adjusts scheme for preCode in DeploySecretContract that comes in as a hex string from core, instead of the byte array.
  • Adjusts the specs for the message to the KeyMgmgt node that is an array, instead of an object
  • And in GetStateKeysAction.js accounts for being called without specifying params which would otherwise throw an error around L21, and unpacks the error object and displays it properly in L45

@Isan-Rivkin Isan-Rivkin requested a review from elichai April 3, 2019 15:26
@Isan-Rivkin
Copy link
Contributor

@elichai please take a look at this

"minItems": 1
}
}
"preCode": {"type": "string"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how did that happen but I think the right thing to do is to change it in core to Vec<u8>

src/core/core_messages_scheme.json Show resolved Hide resolved
src/worker/controller/actions/GetStateKeysAction.js Outdated Show resolved Hide resolved
lacabra added a commit that referenced this pull request Apr 3, 2019
lacabra added a commit that referenced this pull request Apr 3, 2019
@Isan-Rivkin Isan-Rivkin added the not ready Not ready - do not merge yet label Apr 7, 2019
@lacabra
Copy link
Contributor Author

lacabra commented Apr 8, 2019

@Isan-Rivkin, @lenak25: As far as I'm concerned, this is ready to merge because all requested changes have been addressed where relevant. There is an item that @elichai wants to change in core, but in the interest of time, I would merge this as is; and submit a different PR for when core changes. The current version of this PR works with the current version of core. Thanks

@lenak25 lenak25 mentioned this pull request Apr 16, 2019
@lacabra
Copy link
Contributor Author

lacabra commented Apr 25, 2019

closed in favor of #153, #161 and other changes in core that have superseded the changes proposed here.

@lacabra lacabra closed this Apr 25, 2019
@lacabra lacabra deleted the smallAdjustments branch April 25, 2019 17:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
not ready Not ready - do not merge yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants