Document new guidelines (#14309)
This commit is contained in:
Icaro Motta
GitHub
parent
57526a45ad
commit
a77f662948
14
README.md
14
README.md
|
|
@ -22,9 +22,17 @@ If this interests you, **help us make Status a reality** - anyone can contribute
|
|||
|
||||
Go straight to the [docs](https://status.im/docs) or [join our chat](https://join.status.im/chat/public/status) and choose what interests you:
|
||||
|
||||
- **Developer**
|
||||
Developers are the heart of software and to keep Status beating we need all the help we can get! If you're looking to code in ClojureScript or Golang then Status is the project for you! We use React Native and there is even some Java/Objective-C too!
|
||||
Want to learn more about it? Start by reading our [Developer Introduction](https://status.im/developer_tools/) which guides you through the technology stack and start browsing [beginner issues](https://github.com/status-im/status-mobile/issues?utf8=%E2%9C%93&q=is%3Aopen%20is%3Aissue%20label%3A%22good%20first%20issue%22%20). Then you can read how to [Build Status](https://status.im/technical/build_status/), which talks about managing project dependencies, coding guidelines and testing procedures.
|
||||
- **Developer** Developers are the heart of software and to keep Status beating
|
||||
we need all the help we can get! If you're looking to code in ClojureScript or
|
||||
Golang then Status is the project for you! We use React Native and there is even
|
||||
some Java/Objective-C too! Want to learn more about it? Start by reading our
|
||||
[Developer Introduction](https://status.im/developer_tools/) which guides you
|
||||
through the technology stack and start browsing [beginner
|
||||
issues](https://github.com/status-im/status-mobile/issues?utf8=%E2%9C%93&q=is%3Aopen%20is%3Aissue%20label%3A%22good%20first%20issue%22%20).
|
||||
Then you can read how to [Build
|
||||
Status](https://status.im/technical/build_status/), which talks about managing
|
||||
project dependencies, coding guidelines and testing procedures. The [doc/](doc/)
|
||||
directory also has valuable information for contributors.
|
||||
|
||||
- **Community Management**
|
||||
Metcalfe's law states that the value of a network is proportional to the square of the number of connected users of the system - without community Status is meaningless. We're looking to create a positive, fun environment to explore new ideas, experiment and grow the Status community. Building a community takes a lot of work but the people you'll meet and long lasting relationships you form will be well worth it, check out our [Mission and Community Principles](https://status.im/about)
|
||||
|
|
|
|||
|
|
@ -1,5 +1,9 @@
|
|||
# Codebase Structure and Guidelines
|
||||
|
||||
As of Nov/2022, we're starting to document and migrate the codebase to [new
|
||||
guidelines](new-guidelines.md), which will supersede some of the guidelines in
|
||||
this document.
|
||||
|
||||
## Starting the app
|
||||
|
||||
The app is bundled and run in a Nix environment. Information regarding how to run it locally can be found [here](https://status.im/technical/build_status/).
|
||||
|
|
|
|||
|
|
@ -0,0 +1,440 @@
|
|||
# Guidelines
|
||||
|
||||
>The goal of this document is to help all contributors (core and external) to
|
||||
>write code in _unison_ and help establish good practices that serve the Status
|
||||
>Mobile contributors well.
|
||||
|
||||
We don't want to turn this document into an exhaustive list of rules to be
|
||||
followed that nobody will read. As much as possible, we'll try to document only
|
||||
what we consider important for Status Mobile. In other words, we don't want to
|
||||
maintain a general Clojure convention/style guide, nor do we want to turn this
|
||||
document into a long tutorial.
|
||||
|
||||
This is a **work in progress**, and not all conventions are properly implemented
|
||||
in the codebase yet. The project structure is also going over major changes (as
|
||||
of Nov/2022), and it'll take a considerable amount of time until we migrate the
|
||||
existing code to the new structure.
|
||||
|
||||
If you find out anything is outdated or missing, please, share with us or even
|
||||
better, create a pull-request! 🤸
|
||||
|
||||
## Style guide
|
||||
|
||||
We mostly follow the [Clojure Style
|
||||
Guide](https://github.com/bbatsov/clojure-style-guide), so it's recommended to
|
||||
get familiar with it.
|
||||
|
||||
As of Nov/2022, running `make lint` should fix the most basic formatting issues,
|
||||
and we are in the process of integrating a tool to format non-trivial code
|
||||
according to the Clojure Style Guide. This should greatly reduce noise in
|
||||
pull-request reviews and it'll simplify the life of all contributors.
|
||||
|
||||
Pay special attention to:
|
||||
|
||||
- Align let bindings https://github.com/bbatsov/clojure-style-guide#bindings-alignment
|
||||
- Align map keys https://github.com/bbatsov/clojure-style-guide#map-keys-alignment
|
||||
|
||||
## Dos and don'ts
|
||||
|
||||
### Component styles
|
||||
|
||||
Prefer to define styles in separate files named `style.cljs`, usually colocated
|
||||
with the source file using it. For a real example, see
|
||||
[src/status_im/ui2/screens/chat/messages/style.cljs](../src/status_im/ui2/screens/chat/messages/style.cljs).
|
||||
|
||||
```clojure
|
||||
;; bad
|
||||
(defn animated-checkbox-view [{:keys [size]}]
|
||||
[animated/view
|
||||
{:style {:width size
|
||||
:height size
|
||||
:border-radius 4
|
||||
:justify-content :center
|
||||
:align-items :center}}
|
||||
[animated/view
|
||||
(do-something)]])
|
||||
|
||||
;; good
|
||||
(defn animated-checkbox-view [{:keys [size]}]
|
||||
[animated/view
|
||||
{:style (style/animated-checkbox-style size)}
|
||||
[animated/view
|
||||
(do-something)]])
|
||||
```
|
||||
|
||||
### Using TODOs comments
|
||||
|
||||
_TODO_ comments are used extensively in the codebase, but prefer to use them
|
||||
only when strictly necessary and when an issue is not enough to track the work
|
||||
left to be done.
|
||||
|
||||
These are all good examples:
|
||||
|
||||
```clojure
|
||||
;; TODO(@username): <message>
|
||||
;; TODO(@username): <message>, <issue URL>
|
||||
;; TODO(YYYY-MM-DD): <message>
|
||||
;; TODO(@username,YYYY-MM-DD): <message>
|
||||
```
|
||||
|
||||
### Subscription names and event names
|
||||
|
||||
Always register events and subscriptions using a meaningful namespace, but don't
|
||||
namespace them with `::`. We understand it's a controversial decision because
|
||||
there are both pros and cons to such practice.
|
||||
|
||||
Whenever appropriate, it's also recommended to use _fake_ namespaces to convey
|
||||
more knowledge in the keyword about which bounded context (domain) it refers to.
|
||||
You may also use dots to convey hierarchical structures.
|
||||
|
||||
```clojure
|
||||
;; bad
|
||||
;; Don't use real namespaced keywords.
|
||||
(re-frame/reg-sub
|
||||
::profile-pictures-visibility
|
||||
:<- [:multiaccount]
|
||||
(fn [multiaccount]
|
||||
(:profile-pictures-visibility multiaccount)))
|
||||
|
||||
;; good
|
||||
;; Uses a fake namespaced keyword.
|
||||
(re-frame/reg-sub
|
||||
:profile/pictures-visibility
|
||||
:<- [:multiaccount]
|
||||
(fn [multiaccount]
|
||||
(:profile-pictures-visibility multiaccount)))
|
||||
|
||||
;; better
|
||||
;; Uses a fake namespaced keyword with a parent namespace (multiaccount).
|
||||
(re-frame/reg-sub
|
||||
:multiaccount.profile/pictures-visibility
|
||||
:<- [:multiaccount]
|
||||
(fn [multiaccount]
|
||||
(:profile-pictures-visibility multiaccount)))
|
||||
```
|
||||
|
||||
### Declaring view components
|
||||
|
||||
Use the simple `defn` to declare components. Don't use `utils.views/defview` and
|
||||
`utils.views/letsubs`.
|
||||
|
||||
```clojure
|
||||
;; bad
|
||||
(utils.views/defview browser []
|
||||
(utils.views/letsubs [window-width [:dimensions/window-width]]
|
||||
(do-something window-width)))
|
||||
|
||||
;; good
|
||||
(defn browser []
|
||||
(let [window-width (rf/sub [:dimensions/window-width])]
|
||||
(do-something window-width)))
|
||||
```
|
||||
|
||||
### Using re-frame subscriptions and dispatching events
|
||||
|
||||
Use the `utils.re-frame` namespace instead of `re-frame.core` to subscribe and
|
||||
dispatch.
|
||||
|
||||
```clojure
|
||||
;; bad
|
||||
(ns my-namespace
|
||||
(:require [re-frame.core :as rf]))
|
||||
|
||||
(let [username @(rf/subscribe [:username])]
|
||||
[pressable/pressable {:on-press #(rf/dispatch [:do-something])}
|
||||
[rn/view
|
||||
(str "Hello " username)]])
|
||||
|
||||
;; good
|
||||
(ns my-namespace
|
||||
(:require [utils.re-frame :as rf]))
|
||||
|
||||
(let [username (rf/sub [:username])]
|
||||
[pressable/pressable {:on-press #(rf/dispatch [:do-something])}
|
||||
[rn/view
|
||||
(str "Hello " username)]])
|
||||
```
|
||||
|
||||
### Registering event handlers
|
||||
|
||||
Events must always be declared with the `utils.fx/defn` macro. Also, don't use
|
||||
`re-frame.core/reg-event-db`.
|
||||
|
||||
```clojure
|
||||
;; bad
|
||||
(re-frame/reg-event-fx
|
||||
:wakuv2.ui/save-all-confirmed
|
||||
(fn [{:keys [db] :as cofx}]
|
||||
...))
|
||||
|
||||
;; good
|
||||
(fx/defn save-all
|
||||
{:events [:wakuv2.ui/save-all-confirmed]}
|
||||
[{:keys [db] :as cofx}]
|
||||
...)
|
||||
```
|
||||
|
||||
### Registering top-level re-frame subscriptions
|
||||
|
||||
Use `subs.root/reg-root-key-sub` to register top-level (root) subscriptions.
|
||||
Additionally, register root subscriptions in the `subs.root` namespace.
|
||||
|
||||
```clojure
|
||||
;; bad
|
||||
(re-frame/reg-sub
|
||||
:view-id
|
||||
(fn [db]
|
||||
(:view-id db)))
|
||||
|
||||
;; good
|
||||
(reg-root-key-sub :view-id :view-id)
|
||||
```
|
||||
|
||||
### Registering layer-3 subscriptions
|
||||
|
||||
The majority of the subscriptions should be defined as [layer-3
|
||||
subscriptions](https://day8.github.io/re-frame/subscriptions/#the-four-layer)
|
||||
due to performance constraints.
|
||||
|
||||
```clojure
|
||||
;; bad
|
||||
(re-frame/reg-sub
|
||||
:ens/preferred-name
|
||||
(fn [db]
|
||||
(get-in db [:multiaccount :preferred-name])))
|
||||
|
||||
;; good
|
||||
(re-frame/reg-sub
|
||||
:ens/preferred-name
|
||||
:<- [:multiaccount]
|
||||
(fn [multiaccount]
|
||||
(:preferred-name multiaccount)))
|
||||
```
|
||||
|
||||
### Requiring quo2 components
|
||||
|
||||
Consume `quo2` components from `quo2.core`, unless the namespace is also inside
|
||||
the `quo2/` directory.
|
||||
|
||||
```clojure
|
||||
;; bad
|
||||
(ns my-namespace
|
||||
(:require [quo2.components.icon :as icon]))
|
||||
|
||||
(icon/icon :i/verified)
|
||||
|
||||
;; good
|
||||
(ns my-namespace
|
||||
(:require [quo2.core :as quo2]))
|
||||
|
||||
(quo2/icon :i/verified)
|
||||
|
||||
;; also good because both namespaces are inside quo2/
|
||||
(ns quo2.components.tabs.account-selector
|
||||
(:require [quo2.components.markdown.text :as text]))
|
||||
```
|
||||
|
||||
### Require/import
|
||||
|
||||
Prefer `:as` instead of `:refer`. There are exceptions to this rule, e.g. the
|
||||
test macros `deftest` and `is`, which are ubiquitous in the Clojure community.
|
||||
|
||||
```clojure
|
||||
;; bad
|
||||
(ns status-im.utils.datetime
|
||||
(:require [cljs-time.coerce :refer [from-long]]))
|
||||
|
||||
;; good
|
||||
(ns status-im.utils.datetime
|
||||
(:require [cljs-time.coerce :as time.coerce]))
|
||||
```
|
||||
|
||||
### Accessibility labels
|
||||
|
||||
Use keywords instead of strings. As a bonus, remember keywords are cached in
|
||||
memory.
|
||||
|
||||
```clojure
|
||||
;; bad
|
||||
[text/text {:accessibility-label "profile-nickname"}
|
||||
"Markov"]
|
||||
|
||||
;; good
|
||||
[text/text {:accessibility-label :profile-nickname}
|
||||
"Markov"]
|
||||
```
|
||||
|
||||
### Icons
|
||||
|
||||
Use the appropriate keyword qualification/namespace and don't directly require
|
||||
namespaces inside `quo2/components/`.
|
||||
|
||||
```clojure
|
||||
;; bad
|
||||
(require '[quo2.components.icon :as icons])
|
||||
(icons/icon :main-icons2/verified)
|
||||
|
||||
;; good
|
||||
(require '[quo2.core :as quo2])
|
||||
(quo2/icon :i/verified)
|
||||
```
|
||||
|
||||
### Translations
|
||||
|
||||
Prefer to use translation placeholders instead of creating multiple translation
|
||||
keywords and concatenating them into a single string.
|
||||
|
||||
```clojure
|
||||
;; bad
|
||||
;; Assume the translation key is:
|
||||
;; "biometric-auth-error": "Unable perform biometric authentication"
|
||||
(str (i18n/label :t/biometric-auth-error) "(" error-code ")")
|
||||
|
||||
;; good
|
||||
;; Assume the translation key is:
|
||||
;; "biometric-auth-error": "Unable perform biometric authentication ({{code}})"
|
||||
(i18n/label :t/biometric-auth-error {:code error-code})
|
||||
```
|
||||
|
||||
## Project Structure
|
||||
|
||||
First, the bird's-eye view with some example ClojureScript files:
|
||||
|
||||
```
|
||||
src
|
||||
├── js/
|
||||
├── mocks/
|
||||
├── quo2
|
||||
│ ├── components/
|
||||
│ ├── foundations/
|
||||
│ └── theme.cljs
|
||||
├── react_native
|
||||
│ ├── gesture.cljs
|
||||
│ └── platform.cljs
|
||||
├── status_im
|
||||
│ ├── common
|
||||
│ │ └── components
|
||||
│ │ └── bottom_sheet.cljs
|
||||
│ ├── contexts/
|
||||
│ ├── setup/
|
||||
│ └── subs/
|
||||
└── utils.cljs
|
||||
```
|
||||
|
||||
- `src/js`: Raw Javascript files, e.g. React Native Reanimated worklets.
|
||||
- `src/mocks`: Plumbing configuration to be able to run tests.
|
||||
- `src/quo2/`: The component library for Status Mobile.
|
||||
- `src/react_native/`: Contains only low-level constructs to help React Native
|
||||
work in tandem with Clojure(Script).
|
||||
- `src/status_im/common/`: Directories named `common` can appear at any level of
|
||||
the directory tree. Just like directories named `utils`, their directory
|
||||
nesting level communicates their applicable limits.
|
||||
- `src/status_im/common/components/`: Contains reusable components that are not
|
||||
part of the design system (quo2).
|
||||
- `src/status_im/contexts/`: Contains [bounded contexts](#glossary), like
|
||||
`browser/`, `messaging/`, etc. As much as possible, _bounded contexts_ should
|
||||
not directly require each other's namespaces.
|
||||
- `src/status_im/setup/`: Contains namespaces that are mostly used to initialize
|
||||
the application, configure test runners, etc. In general, such namespaces
|
||||
should not be required from the outside.
|
||||
- `src/status_im/subs/`: All subscriptions should live inside it.
|
||||
|
||||
Directories named `utils/` can appear at any level of the directory tree. The
|
||||
directory nesting level precisely indicates its boundaries. For example, a
|
||||
`contexts/user_settings/utils/datetime.cljs` file communicates that it should
|
||||
only be used in the `user_settings` context.
|
||||
|
||||
### src/quo2
|
||||
|
||||
The `src/quo2/` directory holds all components for the new design system. As
|
||||
much as possible, its sub-directories and component names should reflect the
|
||||
same language used by designers.
|
||||
|
||||
Even though the directory lives alongside the rest of the codebase, we should
|
||||
think of it as an external entity that abstracts away particular Status domain
|
||||
knowledge.
|
||||
|
||||
Components inside `src/quo2/` should not rely on re-frame, i.e. they should not
|
||||
dispatch events or use subscriptions.
|
||||
|
||||
Example structure:
|
||||
|
||||
```
|
||||
src
|
||||
└── quo2
|
||||
├── components
|
||||
│ └── dropdown
|
||||
│ ├── style.cljs
|
||||
│ ├── test.cljs
|
||||
│ └── view.cljs
|
||||
└── screens
|
||||
└── dropdown
|
||||
└── view.cljs
|
||||
```
|
||||
|
||||
### Re-frame events
|
||||
|
||||
Event handlers should be defined in files named `events.cljs`, and they should
|
||||
be _close_ to other _things_, like view files, components, etc.
|
||||
|
||||
For example:
|
||||
|
||||
```
|
||||
src
|
||||
└── contexts
|
||||
└── browser
|
||||
├── bookmarks/
|
||||
├── options/
|
||||
├── permissions/
|
||||
├── events.cljs
|
||||
├── events_test.cljs
|
||||
├── style.cljs
|
||||
└── view.cljs
|
||||
```
|
||||
|
||||
### Test structure
|
||||
|
||||
[Unit tests](#glossary) should be created alongside their respective source
|
||||
implementation. We prefer them colocated with the source and not like most
|
||||
Clojure (JVM) codebases which mirror the sources in a top-level test directory.
|
||||
Component tests should be created in a separate directory `__tests__`. Example:
|
||||
|
||||
```
|
||||
├── models
|
||||
│ ├── message.cljs
|
||||
│ └── message_test.cljs
|
||||
├── __tests__
|
||||
│ └── input_bla_test.cljs
|
||||
├── models.cljs
|
||||
└── models_test.cljs
|
||||
```
|
||||
|
||||
There's no hard rule on how integration test namespaces should be split, but
|
||||
we're at least striving to define them under appropriate bounded contexts that
|
||||
mirror the source code.
|
||||
|
||||
```
|
||||
test
|
||||
├── appium/
|
||||
└── integration
|
||||
├── browser/
|
||||
├── communities/
|
||||
├── messaging/
|
||||
├── user_settings/
|
||||
└── wallet
|
||||
└── payment_test.cljs
|
||||
```
|
||||
|
||||
## Glossary
|
||||
|
||||
**Unit test**: The smallest atomic unit that's meaningful to test. For example,
|
||||
tests for utility functions and event handlers are considered unit tests in the
|
||||
mobile codebase. They should be completely deterministic, _fast_, and they
|
||||
should work flawlessly in the REPL.
|
||||
|
||||
**Bounded context**: A logical separation between different domains. It's an
|
||||
important concept in the [Domain-Driven
|
||||
Design](https://en.wikipedia.org/wiki/Domain-driven_design) literature. See
|
||||
[Bounded Context, by Martin
|
||||
Fowler](https://martinfowler.com/bliki/BoundedContext.html) for an introduction
|
||||
to the topic.
|
||||
Loading…
Reference in New Issue