Skip to content
This repository has been archived by the owner on Oct 18, 2023. It is now read-only.

feat: update voting dApp documentation to reflect new js client api #102

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

barthuijgen
Copy link

@barthuijgen barthuijgen commented Jul 24, 2023

I've ran into several issues following the voting dApp guide, and some of them related to the javascript SDK, this turned out to be easier to solve using the upcoming newer version. This MR makes use of @kadena/[email protected] and should not be merged until that release is stable.

It is accompanied by an MR on the kadena-community/voting-dapp repository (link), which also switched to the new Kadena client and updated the frontend project to be more modern.

With these changes it should be possible again to follow along and run the code successfully without running into errors.

@netlify
Copy link

netlify bot commented Jul 24, 2023

Deploy Preview for festive-swartz-adea72 ready!

Name Link
🔨 Latest commit 2fed5e9
🔍 Latest deploy log https://app.netlify.com/sites/festive-swartz-adea72/deploys/64be52150c656a00087779ec
😎 Deploy Preview https://deploy-preview-102--festive-swartz-adea72.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@webpro webpro left a comment

Choose a reason for hiding this comment

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

Nice first PR, but please omit the quoting changes so we can better see what's going on :)

const { PactCommand } = require('@kadena/client');
const { createExp } = require('@kadena/pactjs');
const { PactCommand } = require("@kadena/client");
const { createExp } = require("@kadena/pactjs");
Copy link
Contributor

Choose a reason for hiding this comment

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

When changing the quoting it makes for a hard to review PR. In our JS code we use single quotes, so it makes sense to keep it that way. Feel free to suggest and change this convention, but please don't mix that up in a single PR.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, that's a good suggestion! I'm used to just running prettier, will revert this 👍

@@ -1036,7 +1047,7 @@ Lastly, to get the result of a transaction we are using the `pollTransactions` h
To run the frontend dApp, go to the frontend folder and run:

```shell
npm run start
pnpm run dev
Copy link
Contributor

Choose a reason for hiding this comment

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

People should be free to use any package manager of choice, so I guess it's best to use what everyone already knows in generic community documentation. I.e. users of Yarn, pnpm, bun etc. etc. know how to translate npm commands.

Copy link
Author

Choose a reason for hiding this comment

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

Of course, but in this case we provide an example repository with code and have to make a choice there. This documentation just reflects that. It's not meant to tell people what to use themselves.

npm run dev would also work in a pnpm workspace though, so this could be changed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants