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

feat: integrate matchstick into the graph CLI #1549

Open
wants to merge 122 commits into
base: main
Choose a base branch
from

Conversation

YassinEldeeb
Copy link
Member

No description provided.

Copy link

changeset-bot bot commented Dec 17, 2023

🦋 Changeset detected

Latest commit: cf29a94

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@graphprotocol/graph-cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dimitrovmaksim
Copy link
Member

Hey, you'll also need to integrate matchstick-as as well, which is a helper library similar to graph-ts

Copy link

cloudflare-workers-and-pages bot commented Dec 18, 2023

Deploying graph-tooling with  Cloudflare Pages  Cloudflare Pages

Latest commit: cf29a94
Status: ✅  Deploy successful!
Preview URL: https://da203ca1.graph-tooling.pages.dev
Branch Preview URL: https://add-matchstick.graph-tooling.pages.dev

View logs

packages/matchstick/LICENSE Outdated Show resolved Hide resolved
} = await this.parse(TestCommand);

// Ensure PostgreSQL 14 is installed
try {
Copy link
Member

Choose a reason for hiding this comment

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

nice!

Copy link
Member

Choose a reason for hiding this comment

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

let's update this to real message

GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}

# CLI
Copy link
Member

Choose a reason for hiding this comment

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

uncomment now so we can prep to merge?

@@ -29,12 +29,12 @@
},
"dependencies": {
"@float-capital/float-subgraph-uncrashable": "^0.0.0-alpha.4",
"@graphprotocol/graph-tooling-napi-utils": "workspace:*",
Copy link
Member

Choose a reason for hiding this comment

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

the issue we are facing now is even for test suite we have to build otherwise it will fail 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, but we build it in the CI, maybe locally we should pre-build before testing too

@@ -0,0 +1,43 @@
[package]
name = "matchstick"
Copy link
Member

Choose a reason for hiding this comment

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

rename to napi-utils

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

Comment on lines +39 to +43
# - host: windows-latest
# target: i686-pc-windows-msvc
# build: |
# rustup target add i686-pc-windows-msvc
# pnpm build --target i686-pc-windows-msvc
Copy link
Member

Choose a reason for hiding this comment

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

remove

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