From 8d1b678c42b0726e34e5c80ff1cd16b003a5339d Mon Sep 17 00:00:00 2001 From: 18alantom <2.alan.tom@gmail.com> Date: Tue, 28 Mar 2023 12:05:05 +0530 Subject: [PATCH] chore: add the Code Quality section --- .github/CONTRIBUTING.md | 42 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index f85d5817..58a6a65c 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -3,7 +3,7 @@ If you are a Frappe Books user and want to contribute to improving it _without writing code_, there are several things you can do: -- **Inform us of issues** that you face while using Frappe Books by [raising issues](https://github.com/frappe/books/issues/new) +- **Inform us of issues** you face while using Frappe Books by [raising issues](https://github.com/frappe/books/issues/new). - **Add a language** you would like to use Frappe Books in by [contributing translation](https://github.com/frappe/books/wiki/Contributing-Translations). - **Share your thoughts** on Frappe Books by joining our [Telegram group](https://t.me/frappebooks). - **Use Frappe Books** for your accounting requirements and tell people about it. @@ -14,11 +14,41 @@ If you want to contribute code to Frappe Books, please go through the following - [Code Quality](#code-quality) - [Contributing Features](#contributing-features) + - [Invisible until required](#invisible-until-required) + - [Simple UI](#simple-ui) + - [Documentation and Tests](#documentation-and-tests) + - [PR Description](#pr-description) - [Writing Tests](#writing-tests) ## Code Quality - +A few rules of thumb to ensure that you're contributing maintainable code to Frappe Books: + +- **Readability over succinctness**: If your succinct code takes longer to parse (as + in read and understand) then it is bad code because we aren’t playing code + golf. + - **Write short functions** such that the name of the function accurately describes + what the function does. + - **Use early exits** ([reference](https://softwareengineering.stackexchange.com/questions/18454/should-i-return-from-a-function-early-or-use-an-if-statement)). + - **Don’t nest conditionals and loops**. If you find the need for + nested loops or conditionals, wrap the inner loop or conditional in a function + and call it in the outer code block. + - In general, understand why chunking and naming information is helpful when it + comes to comprehension. +- **Succinctness over readability only if it is significantly more performant**: + For example, if your code goes from `O(n)` to `O(log(n))` then it’s okay to + sacrifice readability. In such a case, add comments that mention what is going + on. +- **Don't Write comments**: Variable names, function names and easy to read code + should do what a comment would. +- **Write comments only if the code can't be explained by its context** such as + if the code is esoteric for the sake of performance. +- **Rebase don't merge**: Merge commits are ugly and should be used only to + merge a large PR. +- **Format your code**: Frappe Books uses `prettier` and `eslint` rules for code + styling and linting, please make sure you have run them and fixed your code + accordingly before pushing. +- **Use TypeScript**: Even the `*.vue` files should use TypeScript ([reference](https://vuejs.org/guide/typescript/overview.html#usage-in-single-file-components)). ## Contributing Features @@ -68,6 +98,14 @@ you to add them for large changes. _Add a link to the documentation PR in your feature PR._ - **Tests**: If your features alters business logic then tests should be added. +### PR Description + +All pull requests should have a meaningful and detailed description. The following things should be in mentioned in the description: + +- **What the change is** should be described in sufficient detail, _not_ a + single line such as _"This PR adds `[some_feature]`"_. +- **Screenshots** should be added if the change affects the UI. + ## Writing Tests You should write tests. If your features alter business logic and there are no