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

Various minor fixes and cleanups for the election dApp tut #319

Merged
merged 2 commits into from
May 30, 2023

Conversation

webpro
Copy link
Contributor

@webpro webpro commented May 24, 2023

Reason:

Closes [issue link]

Changes made (preferably with images/screenshots):

Check off the following:

  • I have reviewed my changes and run the appropriate tests.
  • I have have run rush change to add the appropriate change logs.
  • I have added/edited docs.
  • I have added tutorials.
  • I have double checked and DEFINITELY added docs.

@webpro webpro marked this pull request as ready for review May 24, 2023 10:17
@@ -129,7 +129,7 @@
"Get the votes count by cid"

;; Read the row using cid as key and select only the `votes` column
(at 'votes (read candidates cid ['votes]))
(at 'votes (read candidates cid ["votes"]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Symbols are string literals representing some unique item in the runtime, like a function or a table name. Their representation internally is simply a string literal so their usage is idiomatic.
Symbols are created with a preceding tick, thus they do not support whitespace nor multiline syntax.

Though on most projects I've seen, usually they use a symbol where possible, probably because it's more strict and therefore conveys the idea better? So I'm not entirely sure if we want to change from from ' to "

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The single quote breaks syntax highlighting

Copy link
Contributor

Choose a reason for hiding this comment

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

The single quote breaks syntax highlighting

shouldn't be the case. the atom pact-lang plugin does take it into account and I submitted a patch for the vscode plugin that can be merged too: kadena-community/pact-vscode#3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think in general not closing such strings is confusing, but not gonna argue that here. I reverted this change as it is not a fix like I thought initially.

@webpro webpro merged commit 1a6a117 into master May 30, 2023
@webpro webpro deleted the fix/election-dapp-tut branch May 30, 2023 10:38
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.

7 participants