ValuedMammal commented 3 months ago

Using rustfmt means we always agree on code formatting. If rustfmt allows it, then matters of code style can be left to personal preference.

However we could benefit from defining more code conventions in CONTRIBUTING.md for the sake of consistency. Just some ideas:

Forbid unsafe (or handle case by case)
Use StdExternalCrate style for grouping imports
Return as much context as possible with errors
Refer liberally to bitcoin BIPs and relevant historical context for a piece of code
Follow rust API guidelines for documentation and naming
Name tests after the functional unit they're testing
If using emoji, use them to their intended effect: code-review-emoji-guide

Approving changes:

All comments resolved
2-ACK rule to merge

Use case
Facilitates code review and improves readability of the codebase

Additional context

1221

https://github.com/bitcoindevkit/bdk/pull/1203#discussion_r1510453405

notmandatory commented 3 months ago

Great suggestions!

ValuedMammal commented 3 weeks ago

Provide a reason for ignoring tests. See #1460

oleonardolima commented 3 weeks ago

Concept ACK, thanks for the suggestions!

storopoli commented 3 weeks ago

I really like this, I am totally on-board. Can we discuss this in the next dev meeting? (PS: @ValuedMammal should go first since we almost never go alphabetical descending order) Cc @nondiremanuel

bitcoindevkit / bdk

Have more code conventions and guidelines #1394

1221