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

Improve documentation #1765

Merged
merged 12 commits into from
Jul 7, 2022
Merged

Improve documentation #1765

merged 12 commits into from
Jul 7, 2022

Conversation

turbolent
Copy link
Member

@turbolent turbolent commented Jun 25, 2022

Closes #1701
Closes #1328
Closes #1282
Closes #1237
Closes #1223
Closes #1203
Work towards #959


  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@turbolent turbolent added the Documentation Improvements or additions to documentation label Jun 25, 2022
@turbolent turbolent requested a review from SupunS as a code owner June 25, 2022 22:53
@turbolent turbolent self-assigned this Jun 25, 2022
@codecov
Copy link

codecov bot commented Jun 25, 2022

Codecov Report

Merging #1765 (97af2ff) into master (38706c6) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1765      +/-   ##
==========================================
- Coverage   77.43%   77.43%   -0.01%     
==========================================
  Files         306      305       -1     
  Lines       62442    62552     +110     
==========================================
+ Hits        48355    48437      +82     
- Misses      12467    12494      +27     
- Partials     1620     1621       +1     
Flag Coverage Δ
unittests 77.43% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
runtime/common/bigInt.go 47.82% <0.00%> (-52.18%) ⬇️
runtime/interpreter/debugger.go 73.21% <0.00%> (-16.79%) ⬇️
runtime/ast/expression.go 88.82% <0.00%> (-0.99%) ⬇️
runtime/sema/type.go 89.25% <0.00%> (-0.30%) ⬇️
runtime/repl.go 0.00% <0.00%> (ø)
runtime/stdlib/crypto.go 85.26% <0.00%> (ø)
runtime/interpreter/value.go 72.24% <0.00%> (ø)
runtime/stdlib/hashalgorithm.go 90.76% <0.00%> (ø)
runtime/stdlib/signaturealgorithm.go 100.00% <0.00%> (ø)
runtime/parser2/lexer/token.go
... and 49 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 38706c6...97af2ff. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Jun 25, 2022

Cadence Benchstat comparison

This branch with compared with the base branch onflow:master commit 3db7348
The command for i in {1..N}; do go test ./... -run=XXX -bench=. -benchmem -shuffle=on; done was used.
Bench tests were run a total of 7 times on each branch.

Collapsed results for better readability

old.txtnew.txt
time/opdelta
CheckContractInterfaceFungibleTokenConformance-2134µs ± 6%132µs ± 2%~(p=0.805 n=7+7)
ContractInterfaceFungibleToken-242.0µs ± 7%41.6µs ± 1%~(p=0.268 n=7+5)
InterpretRecursionFib-23.65ms ± 1%3.66ms ± 0%~(p=0.177 n=6+5)
NewInterpreter/new_interpreter-21.11µs ± 0%1.17µs ± 2%+4.96%(p=0.001 n=7+6)
NewInterpreter/new_sub-interpreter-22.43µs ± 1%2.50µs ± 1%+2.75%(p=0.003 n=7+5)
ParseArray-28.38ms ± 5%8.80ms ± 2%+4.93%(p=0.014 n=7+6)
ParseDeploy/byte_array-212.4ms ± 2%13.3ms ± 1%+6.80%(p=0.003 n=7+5)
ParseDeploy/decode_hex-21.12ms ± 1%1.21ms ± 7%~(p=0.051 n=7+6)
ParseFungibleToken/With_memory_metering-2200µs ± 1%214µs ± 3%+6.93%(p=0.002 n=6+6)
ParseFungibleToken/Without_memory_metering-2152µs ± 1%163µs ± 3%+7.29%(p=0.001 n=6+7)
ParseInfix-27.56µs ± 1%8.05µs ± 7%+6.43%(p=0.033 n=6+7)
QualifiedIdentifierCreation/One_level-23.00ns ±20%3.05ns ±18%~(p=0.343 n=7+7)
QualifiedIdentifierCreation/Three_levels-2119ns ± 0%136ns ± 3%+14.18%(p=0.002 n=6+6)
RuntimeFungibleTokenTransfer-2976µs ±25%990µs ±27%~(p=0.318 n=7+7)
RuntimeResourceDictionaryValues-25.99ms ± 1%6.19ms ± 4%+3.22%(p=0.022 n=6+7)
ValueIsSubtypeOfSemaType-281.7ns ± 1%90.0ns ± 2%+10.20%(p=0.002 n=6+6)
 
alloc/opdelta
CheckContractInterfaceFungibleTokenConformance-267.0kB ± 0%67.0kB ± 0%~(p=1.000 n=7+7)
ContractInterfaceFungibleToken-226.8kB ± 0%26.8kB ± 0%~(all equal)
InterpretRecursionFib-21.51MB ± 0%1.51MB ± 0%~(p=0.636 n=5+7)
NewInterpreter/new_interpreter-2928B ± 0%928B ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-21.46kB ± 0%1.46kB ± 0%~(all equal)
ParseArray-22.80MB ± 2%2.82MB ± 2%~(p=0.209 n=7+7)
ParseDeploy/byte_array-24.44MB ± 2%4.42MB ± 3%~(p=0.971 n=7+7)
ParseDeploy/decode_hex-2214kB ± 0%214kB ± 0%~(p=0.860 n=7+7)
ParseFungibleToken/With_memory_metering-236.3kB ± 0%36.3kB ± 0%~(p=0.429 n=7+7)
ParseFungibleToken/Without_memory_metering-236.3kB ± 0%36.3kB ± 0%~(p=0.714 n=7+7)
ParseInfix-22.17kB ± 0%2.17kB ± 0%~(p=0.125 n=7+7)
QualifiedIdentifierCreation/One_level-20.00B 0.00B ~(all equal)
QualifiedIdentifierCreation/Three_levels-264.0B ± 0%64.0B ± 0%~(all equal)
RuntimeFungibleTokenTransfer-2232kB ± 0%232kB ± 1%~(p=0.902 n=7+7)
RuntimeResourceDictionaryValues-22.25MB ± 0%2.25MB ± 0%~(p=0.805 n=7+7)
ValueIsSubtypeOfSemaType-248.0B ± 0%48.0B ± 0%~(all equal)
 
allocs/opdelta
CheckContractInterfaceFungibleTokenConformance-21.08k ± 0%1.08k ± 0%~(all equal)
ContractInterfaceFungibleToken-2461 ± 0%461 ± 0%~(all equal)
InterpretRecursionFib-233.5k ± 0%33.5k ± 0%~(all equal)
NewInterpreter/new_interpreter-213.0 ± 0%13.0 ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-242.0 ± 0%42.0 ± 0%~(all equal)
ParseArray-270.0k ± 0%70.0k ± 0%~(p=0.592 n=7+7)
ParseDeploy/byte_array-2105k ± 0%105k ± 0%~(p=0.592 n=7+7)
ParseDeploy/decode_hex-277.0 ± 0%77.0 ± 0%~(all equal)
ParseFungibleToken/With_memory_metering-21.06k ± 0%1.06k ± 0%~(all equal)
ParseFungibleToken/Without_memory_metering-21.06k ± 0%1.06k ± 0%~(all equal)
ParseInfix-266.0 ± 0%66.0 ± 0%~(all equal)
QualifiedIdentifierCreation/One_level-20.00 0.00 ~(all equal)
QualifiedIdentifierCreation/Three_levels-22.00 ± 0%2.00 ± 0%~(all equal)
RuntimeFungibleTokenTransfer-24.19k ± 0%4.19k ± 0%~(p=1.000 n=7+7)
RuntimeResourceDictionaryValues-237.5k ± 0%37.5k ± 0%~(p=0.735 n=7+7)
ValueIsSubtypeOfSemaType-21.00 ± 0%1.00 ± 0%~(all equal)
 

@@ -198,7 +198,8 @@ struct AccountKey {
let isRevoked: Bool
}
```
Refer the [PublicKey](crypto/#publickey) section for more details on the creation and validity of public keys.

Refer the [`PublicKey` section](crypto/#publickey) for more details on the creation and validity of public keys.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Refer the [`PublicKey` section](crypto/#publickey) for more details on the creation and validity of public keys.
Refer to the [`PublicKey` section](crypto/#publickey) for more details on the creation and validity of public keys.

```

It is invalid to declare a synthetic field with only a setter.
Initializers do not support overloading.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to add some language here to call out that we plan on supporting this in the future? While documenting the behavior as if it exists is misleading, a statement like this may give the impression that we have no intention of ever supporting overloading at all. Perhaps something like "Initializers do not currently support overloading, but there are plans to add support for this in the future."

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. There aren't any plans to add support for overloading at the moment, it's just a feature I had thought we would want when writing this 3 years ago, and since then there has been no mention/discussion around it

}
}
```
Functions do not support overloading.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comments here as about initializers.

docs/language/core-events.md Outdated Show resolved Hide resolved
docs/language/core-events.md Outdated Show resolved Hide resolved
docs/language/operators.md Outdated Show resolved Hide resolved
docs/language/operators.md Outdated Show resolved Hide resolved
docs/language/operators.md Outdated Show resolved Hide resolved
docs/language/operators.md Outdated Show resolved Hide resolved
docs/language/operators.md Outdated Show resolved Hide resolved
docs/language/accounts.mdx Outdated Show resolved Hide resolved
docs/language/operators.md Outdated Show resolved Hide resolved
@turbolent turbolent requested a review from wise4rmgod June 27, 2022 21:56
@turbolent turbolent mentioned this pull request Jul 6, 2022
6 tasks
Copy link
Member

@joshuahannan joshuahannan left a comment

Choose a reason for hiding this comment

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

Looks good!

docs/language/core-events.md Outdated Show resolved Hide resolved
@turbolent turbolent merged commit 86b481e into master Jul 7, 2022
@turbolent turbolent deleted the bastian/improve-documentation branch July 7, 2022 19:48
turbolent added a commit that referenced this pull request Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements or additions to documentation
Projects
None yet
3 participants