best practices update #921
Conversation
|
|
||
| ## Code Placement | ||
|
|
||
| The lib.rs file should contain only the init and upgrade functions most of the time. Sometimes it's tempting to bundle a bunch of unrelated functions there, but don't. You'll end up with a lib.rs file of 500 lines. |
There was a problem hiding this comment.
Code formatting, NTH. E.g. init instead of init etc.
|
|
||
| ## Code Placement | ||
|
|
||
| The lib.rs file should contain only the init and upgrade functions most of the time. Sometimes it's tempting to bundle a bunch of unrelated functions there, but don't. You'll end up with a lib.rs file of 500 lines. |
There was a problem hiding this comment.
of 500 lines -> be less specific e.g. "with a unreasonably large file".
|
|
||
| ## Function Size | ||
|
|
||
| Each function should be 30-50 lines. Any more than that and it's really hard to navigate the file. |
There was a problem hiding this comment.
... at most 30-50 (actually, even 30-line functions are extremely large).
|
|
||
| ## Module Placement | ||
|
|
||
| Each module can be placed in its own folder along with other related modules. Sure, splitting the code is nice, but having to navigate through 20 files, all at the level of lib.rs, doesn't help at all. This makes it even easier to search for specific features. |
There was a problem hiding this comment.
through 20 - be less specific e.g. "through a large number of files".
doesn't help at all - maybe replace with "might be cumbersome" (somehow friendlier).
|
|
||
| ## Error Messages | ||
|
|
||
| If you have the same error message in multiple places, it's better to declare a static with the message and use that instead of copy-pasting the message. If you have the same condition too, consider having a separate `require_X` function. |
There was a problem hiding this comment.
A short snippet below the guideline would be helpful.
|
|
||
| ## Small PRs | ||
|
|
||
| Unless you're mass-upgrading everything, in which case you really have no other choice, it's much better to keep your PRs focused on one specific task. Also much easier for reviewers to spot issues instead of simply giving you a green and moving on. |
There was a problem hiding this comment.
Maybe drop the "instead of simply giving you a green and moving on" - its' sufficient to say: "Also it's much easier for reviewers to spot issues".
Description of the pull request (what is new / what has changed)
Did you test the changes locally ?
Which category (categories) does this pull request belong to?