Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

Conversation

@mikeseese
Copy link
Contributor

@mikeseese mikeseese commented Feb 4, 2021

This PR is a continuation of both #767 (which was a continuation of #726) and #712. Here is what this PR adds:

  • Add the Filecoin package to the CLI interface properly
    • This includes keeping the @ganache/filecoin package as a peerDependency for @ganache/flavors and failing if the package cannot be found
    • This peer dependency architecture required me to provide separate type declarations to prevent a circular dependency. I have a strong feeling @davidmurdoch will not be a fan of this, but I believe this is the only setup that will work in production. I could be wrong and am open to alternatives. I provided a root script tsc.filecoin.declarations to allow for quick ease and file cleaning via pretty-quick
    • Neither of these two things (root linking script and type declaration generation) are currently automated (meaning the developer has to call them when they need it). I'm not sure if/who you want this in your development process, so I left it out as it's a trivial thing to setup later. I can do this if you let me know what the desired dev experience should be.
  • Preliminary reorganization of tests in @ganache/filecoin. This branch also fixes existing tests.
  • Some other minor housekeeping stuff (some code styling, etc)
  • Supports the Chain Explorer feature in the https://github.com/filecoin-shipyard/filecoin-network-inspector example
  • Adds support for truffle preserve
  • Corrects all of the Filecoin types to be consistent with the Filecoin API v1.4.0

It working with the Chain Explorer:
image

It working with truffle preserve:
image.png

@mikeseese mikeseese requested a review from tcoulter February 4, 2021 19:04
@mikeseese mikeseese self-assigned this Feb 4, 2021
@mikeseese mikeseese changed the title feat: add filecoin to CLI, Filecoin.ChainNotify metho, and Filecoin support for websockets/subscriptions feat: add filecoin to CLI, Filecoin.ChainNotify method, and Filecoin support for websockets/subscriptions Feb 4, 2021
Copy link
Contributor

@davidmurdoch davidmurdoch left a comment

Choose a reason for hiding this comment

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

This is a partial review. I just want to get this feedback to you sooner rather than later.

@mikeseese mikeseese force-pushed the feat/filecoin-chainnotify branch from e6da8f8 to ec414e0 Compare February 5, 2021 04:29
Copy link
Contributor

@tcoulter tcoulter left a comment

Choose a reason for hiding this comment

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

Lots of awesome work here, and it's great to see this progressing. Aside, it's super cool you got Websockets in there.

I made a handful of comments. The majority of them are related to code style for one specific pattern you use within the code. Would love for you to change that. The rest are questions.

Thanks again!

@mikeseese mikeseese requested a review from tcoulter February 5, 2021 19:44
@mikeseese mikeseese force-pushed the feat/filecoin-chainnotify branch from c5a0cef to 96a633a Compare February 5, 2021 20:14
@mikeseese
Copy link
Contributor Author

@tcoulter approved this on Slack

@mikeseese mikeseese merged commit 5972ff0 into filecoin Feb 11, 2021
@mikeseese mikeseese deleted the feat/filecoin-chainnotify branch February 11, 2021 21:08
mikeseese added a commit that referenced this pull request Feb 16, 2021
…n support for websockets/subscriptions (#768)

Co-authored-by: David Murdoch <[email protected]>
mikeseese added a commit that referenced this pull request Mar 25, 2021
…n support for websockets/subscriptions (#768)

Co-authored-by: David Murdoch <[email protected]>
mikeseese added a commit that referenced this pull request Mar 30, 2021
…n support for websockets/subscriptions (#768)

Co-authored-by: David Murdoch <[email protected]>
mikeseese added a commit that referenced this pull request Mar 30, 2021
…n support for websockets/subscriptions (#768)

Co-authored-by: David Murdoch <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants