|
| 1 | +This document details steps to improve codebase intelligibility. Codebase should be as simple and intuitive as possible to understand and navigate. |
| 2 | +It goes in pair with a best practices document that will follow. |
| 3 | + |
| 4 | +status: RFC |
| 5 | + |
| 6 | +To reduce potential for regression and simplify implementation and subsequent merge into develop this refactoring can be split into multiple PRs |
| 7 | + |
| 8 | +# Pull request #1 Cosmetic |
| 9 | + |
| 10 | +This PR only moves things around and rename things so it should not introduce any behavior changes |
| 11 | + |
| 12 | +## Introduce new hierarchy |
| 13 | + |
| 14 | +Reorganize folders / rename file to match new hierarchy and match re-frame semantic. |
| 15 | + |
| 16 | +``` |
| 17 | +src/status-im/ |
| 18 | +├── components |
| 19 | +├── data_store |
| 20 | +├── platforms |
| 21 | +├── protocol |
| 22 | +├── translations |
| 23 | +├── ui |
| 24 | +│ └── screens |
| 25 | +│ ├── account |
| 26 | +│ │ ├── login |
| 27 | +│ │ └── recover |
| 28 | +│ │ ├── core.cljs |
| 29 | +│ │ ├── db.cljs |
| 30 | +│ │ ├── events.cljs |
| 31 | +│ │ ├── subs.cljs |
| 32 | +│ │ └── views.cljs |
| 33 | +│ ├── contacts |
| 34 | +│ │ └── new_contact |
| 35 | +│ │ └── views.cljs |
| 36 | +│ └── navigation.cljs |
| 37 | +└── utils |
| 38 | +``` |
| 39 | +and |
| 40 | +``` |
| 41 | +resources |
| 42 | +├── default_contacts.json |
| 43 | +├── default_contacts_groups.json |
| 44 | +├── images/ |
| 45 | + | └── contacts/ |
| 46 | +├── bots/ |
| 47 | + | ├── browse/ |
| 48 | + | ├── ... |
| 49 | + | └── console/ |
| 50 | +└── vendors/ |
| 51 | +└── js/ |
| 52 | +``` |
| 53 | + |
| 54 | +- if you have specs in a specs.cljs file move them to the db.cljs file |
| 55 | + |
| 56 | +## Apply code conventions |
| 57 | + |
| 58 | +Fix code convention according to best practices. |
| 59 | + |
| 60 | +https://github.com/bbatsov/clojure-style-guide |
| 61 | +http://tonsky.me/blog/readable-clojure/ |
| 62 | + |
| 63 | +- Use long namespace aliases (i.e use `string` rather than `str` or `s` for `clojure.string`) |
| 64 | +- Use `react` alias instead of directly refering to components in views |
| 65 | + |
| 66 | +The important thing is that by following such guidelines the way to refer to and name things should be more consistent accross the codebase. |
| 67 | + |
| 68 | +### Add letsubs macro in views |
| 69 | + |
| 70 | +#### For emacs users |
| 71 | + |
| 72 | +You can use the following emacs-lisp expression in your .emacs to have proper indentation with the letsubs macro |
| 73 | + |
| 74 | +```emacs-lisp |
| 75 | + (put-clojure-indent 'letsubs 1) |
| 76 | +``` |
| 77 | + |
| 78 | +#### For cursive users |
| 79 | + |
| 80 | +### Write docstring for important functions |
| 81 | + |
| 82 | +# Pull request #2 Functional |
| 83 | + |
| 84 | +## Rewrite events |
| 85 | + |
| 86 | +### Use fx and cofx |
| 87 | +### Move navigation/preload-data! to navigation.cljs |
| 88 | + |
| 89 | +## Refactor app-db |
| 90 | + |
| 91 | +Make sure app-db is |
| 92 | + |
| 93 | +- properly normalized |
| 94 | +- using namespaced keys |
| 95 | +- speced |
| 96 | +- documented. |
| 97 | + |
| 98 | +From @janherich |
| 99 | + |
| 100 | +Cases which I find problematic: |
| 101 | + |
| 102 | +Data under top level keys :chat-animations, :chat-loaded-callbacks, :chat-ui-props and :chats should me merged together, as they are all indexed by chat-id, I propose top level key :chat-by-id |
| 103 | +Messages should be kept at top level key :message-by-id, every occurrence where message is referenced (for example [:chat-by-id "console" :messages] will contain collection of references to particular messages, like [:message-by-id "intro-message"] we can then either lookup data manually in subscriptions by using reference path as simple (get-in db ...) argument, or maybe even provide some automatic denormalisation where all references (links) will be replaced be referenced data in subscriptions, I'm not sure how that's feasible in reagent subscription model. |
| 104 | + |
| 105 | +## Rewrite subs |
| 106 | + |
| 107 | +Use re-frame best practices: |
| 108 | + |
| 109 | +- db can be used only in extraction subscriptions |
| 110 | + |
| 111 | + |
| 112 | +## Write tests |
| 113 | + |
| 114 | +# Pull request #3 Misc |
| 115 | + |
| 116 | +## Consolidate namespaces |
| 117 | + |
| 118 | +Merge utils namespaces and reduce files number to improve code understandability. |
| 119 | +Consolidate android and ios shared code. |
| 120 | + |
| 121 | +## Improve modularization |
| 122 | + |
| 123 | +Make sure we don't have unwanted dependencies between namespaces (i.e. re-frame usage in non ui/ namespaces) |
| 124 | +Might require spliting files |
| 125 | + |
| 126 | + |
| 127 | +## Follow re-frame patterns |
| 128 | + |
| 129 | +Reorganize ui code by views and follow re-frame patterns |
| 130 | + |
0 commit comments