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

Start reviewing Primitive.Types docs #243

Merged
merged 1 commit into from
May 10, 2019
Merged

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented May 8, 2019

Issue Number

#220

Overview

  • Simpler module doc comment
  • Doc comment for WalletMetadata and Direction

Comments

@@ -399,10 +399,10 @@ instance Buildable TxStatus where
InLedger -> "in ledger"
Invalidated -> "invalidated"

-- | The flow of funds in to or out of a wallet.
-- | The @Direction@ of a transaction - will the wallet balance decrease or not?
Copy link
Member Author

Choose a reason for hiding this comment

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

According to cardano-sl:

A transaction is outgoing when it decreases the wallet’s balance

Copy link
Contributor

Choose a reason for hiding this comment

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

The constructor field comments are good. The rhetorical question in the data type comment is not good. I like "The flow of funds in to or out of a wallet", or "The effect of a Transaction on the wallet balance."

@@ -146,6 +144,8 @@ import qualified Data.Text.Encoding as T
Wallet Metadata
-------------------------------------------------------------------------------}

-- | @WalletMetadata@ stores additional information that is of interest to the
-- API.
Copy link
Member Author

Choose a reason for hiding this comment

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

interest to the API

It seemed to me like this was the most honest way of describing it as we don't really care about it in the Primitive/ folder

Copy link
Contributor

Choose a reason for hiding this comment

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

It's the place where we store information about the wallet which isn't transactional, isn't key related, and isn't updated every time a block is applied. (Actually the WalletState is updated after applying blocks - maybe this field can be easily calculated anyway).

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting point, thanks.

I suspect the same would go for delegation

@Anviking Anviking self-assigned this May 8, 2019
@Anviking Anviking force-pushed the anviking/220/primitive-type-docs branch from 02ca8ca to 812c768 Compare May 8, 2019 12:10
@Anviking Anviking changed the title Start improving Primitive.Types docs Start reviewing Primitive.Types docs May 8, 2019
@Anviking Anviking force-pushed the anviking/220/primitive-type-docs branch 3 times, most recently from ec359de to 1cfb28b Compare May 10, 2019 18:37
@Anviking Anviking force-pushed the anviking/220/primitive-type-docs branch from 1cfb28b to 0d18b0e Compare May 10, 2019 18:52
@Anviking Anviking requested a review from KtorZ May 10, 2019 18:55
@KtorZ KtorZ merged commit 54c6fe9 into master May 10, 2019
@KtorZ KtorZ deleted the anviking/220/primitive-type-docs branch May 10, 2019 20:05
@KtorZ
Copy link
Member

KtorZ commented May 10, 2019

Looks good :)

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