• Designs that separate concerns

• Readability, simplicity, and clarity of code and other technical products

• Appropriate levels of documentation

• Do include a current copyright and license notice at the top of every LiteFarm source file, using the format shown below.

• Don't include the filename in the notice.

Notes

1. At the time of this writing, not a single file in LiteFarm fully meets this expectation. While writing the guidance, we learned that using year ranges like 2019-2022 is not ideal.

2. Each subsequent year that a LiteFarm version is released will be added to the year list in future. When all files are following best practice, this could be automated.

3. Currently, many LF files have no notice at all.

4. Currently, many LF files incorrectly state that the software is copyrighted by the Free Software Foundation. (It is the license text that is copyrighted by FSF.)

5. Currently, many LF files include a filename in the second line of text. Often the filename is incorrect because of renaming files or copy/pasting the notice. Do not include a filename.

Format:

/*
 *  Copyright 2023 LiteFarm.org
 *  This file is part of LiteFarm.
 *
 *  LiteFarm is free software: you can redistribute it and/or modify
 *  it under the terms of the GNU General Public License as published by
 *  the Free Software Foundation, either version 3 of the License, or
 *  (at your option) any later version.
 *
 *  LiteFarm is distributed in the hope that it will be useful,
 *  but WITHOUT ANY WARRANTY; without even the implied warranty of
 *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 *  GNU General Public License for more details, see <https://www.gnu.org/licenses/>.
 */

• Do use comments in JSDoc format to the greatest extent reasonable.

Notes

1. See A tour of the codebase and technical stack for examples that include the tags @async, @desc, @enum, @link, @member, @name, @param, @returns, @see, @static.

2. There is a preferred style for writing the initial description (the first part of the comment, with an implied @desc tag.) Try to emulate the examples; sticklers can consult the grammar-oriented rules here.

3. LF does not actually run the jsdoc to to generate formatted documentation, and possibly never will. This is still a good format for internal documentation.

• Do use comments to clarify intent, where JSDoc comments are not appropriate (inside functions for example).

• Don't make others figure out what you are doing or why.

"Good comments don't repeat the code or explain it. They clarify its intent. Comments should explain, at a higher level of abstraction than the code, what you're trying to do." Steve McConnell, Code Complete

Do resolve warning and error messages reported by development and operations tools to the greatest extent reasonable.

Notes

1. There are currently many warning or error messages from compilers, browser consoles, etc.

2. While it may be “safe” to ignore most of them, that’s a very slippery slope. In effect you end up ignoring all of them, including any new ones that might be buried in the pile.

3. Ideally, messages are resolved by improving the code. In some situations it may be possible and appropriate to “disable” the message. This should be done with caution. The disabling should be for the smallest scope possible. A comment should indicate why the check was disabled rather than changing the code.

4. When there is consensus to delay this work to the future, the work should be captured in new Jira ticket(s) that document this technical debt.

• Don't write code using the moment.js library.

• Do use alternatives.

• Do refactor code using moment.js to use alternatives, especially JavaScript features that need no custom library.

Notes

1. LF uses the moment.js library widely.

2. The maintainers of moment.js have explained reasons behind this, and alternatives available.

• Do prefer camelCaseNames to snake_case or other alternatives in JavaScript, where feasible.

• Don't use camel case in Postgres because it is not well supported; use snake case.

Notes

1. Postgres names for tables, columns, etc. are primarily in snake_case, and there is no intent to change this. Postgres has strange limited support for camel case, apparently related to capitalization. (Postgres will drive you crazy telling you that table userFarm does not exist; you must quote it as "userFarm").

2. Postgres names will necessarily appear in some JavaScript code, so there will always be exceptions to this general guidance.

3. Database access libraries, including the ones in LF, typically provide ways of transforming names, so that querying a table for columns like user_id and farm_id can return JavaScript objects with properties userId and farmId. These features are not currently used, but there is ambition to do so.

4. In the meantime, where you are free to choose names in JavaScript code, use camel case.

• Do reduce nesting as much as reasonably possible to prioritize simplicity, readability, and clarity of code.

Notes

1. At the time of this writing, LiteFarm code has not been systematically examined with respect to this guidance.

2. While most developers would probably agree with the general rule, the "early return" pattern is one specific scenario where opinions diverge. This article does a fair job of weighing the pros and cons; subject to future team discussion, we will follow its conclusion and favor early returns to reduce nesting.

3. Jest and similar test tools often are used to write deeply nested test code, and this is true of many LF backend tests. While this exception to the guidance is perhaps more acceptable than most, at least consider de-nesting test code.

Every new view that is created should have a corresponding story in Storybook. To do that, create a new folder and corresponding file that ends in ".stories.js" in the stories folder, and import the pure component in that file.

• Do include cypress tests with all new frontend features.

• Do add cypress tests for existing frontend code to the greatest reasonable extent.

Notes

1. See the notes on test coverage under jest-tests.

• Don't use string literals for text that will be visible to the user.

• Do use the established internationalization approach based on translation keys.

• Do include all supported languages when adding translation keys.

• Do use 'MISSING' as the translation where you are unable to supply the correct translation. Translators will search and resolve these translations at a later point in time.

• Do include jest tests with all new backend features-- especially all new routes/endpoints.

• Do add jest tests for existing backend code to the greatest reasonable extent.

Notes

1. Our short-term goal is to reach 70% test coverage for LF code. New development should exceed this measure. Tests for existing code may need expanded to reach it. When there is consensus to delay this expansion to the future, the work should be captured in new Jira ticket(s) that document this technical debt.

• Don't define middleware or route handlers by calling a function that always returns the same function.

• Do use configurable middleware (see bottom of this page) when needed.

Notes

1. Express routing is generally defined by naming callback functions that will be called when requests occur. This is the case for the final, route handling function in router.get('/', checkUserFarmStatus(), NotificationUserController.getNotifications);

2. LF often defines Express routing with embedded calls to functions that return callback functions. When parameters are passed, this is typically an appropriate use of configurable middleware, as in many calls to hasFarmAccess and checkScope. When no paramaters are passed to the embedded fuction call, this is occasionally an appropriate use of configurable middleware that relies on a default parameter value, as in the call to checkUserFarmStatus() in the previous note's example. Because of a default parameter value, that call is equivalent to checkUserFarmStatus('Active').

3. But most often, embedded function calls without parameters are like router.get('/', rolesController.getRoles());, where the function called is defined as

getRoles() {
  return async (req, res) => {
    /* statements that handle request */
  }
}

This is a needless level of complexity. It would be better to refactor the router.get statement to simply name the route handler without calling it, and simplify the definition of getRoles to be:

async getRoles(req, res) {
  /* statements that handle request */
}

• Do use Express middleware for recurring logic that requires using the methods of the req and res objects.

• Don't use middleware when the route handler could simply pass data (including values within req and res) to an ordinary function.

Notes

1. Express middleware is a flexible, powerful mechanism with appropriate uses.

2. It introduces an unusual control structure for execution flow that requires the reader to understand Express and to keep in mind details that are not present in the route handling code.

3. Middleware can often be avoided in favor of ordinary functions called from the route handling code. The guidance suggests some possible criteria for deciding.

• Do keep all API code that depends upon database details within model classes.

• Don't use Knex or SQL in controllers, middleware, or any API code outside of models.

• Don't use Objection API features that do much more than CRUD operations anywhere outside of models.

Notes

1. This is a special instance of the "separate concerns" priority.

2. For this discussion, migrations are not strictly part of API code. They necessarily involve database details outside of models.

3. Most LF code was not written with this in mind-- a situation that needs improvement. Contrast this with the controller code in A tour of the codebase and technical stack.

4. It is appropriate for controllers to call CRUD operations like the Objection update method used in NotificationUserController.patchNotifications. But it is inappropriate for controllers to use Knex or low-level Objection features that depend on database details. The NotificationUser model defines a getNotificationsForFarmUser() method that hides these details from the controller's getNotifications() route handler.