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

feat: add filecoin to dynamic cli #767

Closed
wants to merge 32 commits into from

Conversation

mikeseese
Copy link
Contributor

@mikeseese mikeseese commented Feb 4, 2021

This PR is a continuation of the work that was done in #726, but it includes more. Here's the full list of this PR's intention:

  • 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
    • Due to the above, a link.filecoin script was added to the root package which is meant as a development helper to link the filecoin package for usage
    • This peer dependency architecture also 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. I originally intended to fix tests, but I found myself spending too much time fixing tests that were no longer fully applicable due to large overhauls in @ganache/filecoin/src/things. I didn't want to spend much time getting the tests to pass in this state. You can see tests are passing in the subsequent branch. I strongly encourage we do not fixate on tests that are failing prior to this branch when this branch does not alter the behavior behind those tests.
  • Some other minor housekeeping stuff (some code styling, etc)

mikeseese and others added 30 commits January 29, 2021 10:36
"test": "nyc --reporter lcov npm run mocha",
"mocha": "cross-env TS_NODE_COMPILER=ttypescript TS_NODE_FILES=true mocha --exit --check-leaks --throw-deprecation --trace-warnings --require ts-node/register --timeout 10000 'tests/**/*.test.ts'"
"mocha": "cross-env TS_NODE_COMPILER=ttypescript TS_NODE_FILES=true ts-node ../../../../scripts/lerna-mocha.ts --require ts-node/register '_PACKAGEDIR_/tests/**/*.test.ts'"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the filecoin branches are currently based off of #757 as it helps my development process. This adheres to that if it is accepted. It will be changed if #757 is rejected.

@mikeseese
Copy link
Contributor Author

mikeseese commented Feb 4, 2021

I'm closing this PR because @tcoulter wants PRs that have passing tests. Instead we are creating a PR with the next branch (feat/filecoin-chainnotify) that targets filecoin (so it'll also include this work)

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.

1 participant