code.rst 18 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354355356357358359360361362363364365366367368369370371372373374375376377378379380381382383384385386387388389390391392393394395396397398399400401402403404405406407408409410411412413414415416417418419420421422423424425426427428429430431432433434435436437438439440441442443444445446447448449450451452453454455456457458459460461462463464465466467468469470471472473474475476477478479480481482483484485486487488489490491492493494495496497498499500501502503504505506507508509510511512513514515516517518519520521522523524525526527528529530531532533534535536537538539540541542543544545546547548549550551552553554555556557558559560561562563564565566567568569570571572573574575576577578579580581582583584585586587588589590591592593594595596597598599600601602603604605606607608609610611612613614615616617618619620621622623624625626627628629630631632633
  1. Writing Code
  2. ############
  3. Understand Elgg's standards and processes to get your changes accepted as quickly as possible.
  4. .. contents:: Contents
  5. :local:
  6. :depth: 1
  7. License agreement
  8. =================
  9. By submitting a patch you are agreeing to license the code
  10. under a `GPLv2 license`_ and `MIT license`_.
  11. .. _GPLv2 license: http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
  12. .. _MIT license: http://en.wikipedia.org/wiki/MIT_License
  13. Pull requests
  14. =============
  15. Pull requests (PRs) are the best way to get code contributed to Elgg core.
  16. The core development team uses them even for the most trivial changes.
  17. For new features, `submit a feature request <https://github.com/Elgg/Elgg/issues>`__ or `talk to us`_ first and make
  18. sure the core team approves of your direction before spending lots of time on code.
  19. .. _talk to us: http://community.elgg.org/groups/profile/211069/feedback-and-planning
  20. Checklists
  21. ----------
  22. Use these markdown checklists for new PRs on github to ensure high-quality contributions
  23. and help everyone understand the status of open PRs.
  24. Bugfix PRs:
  25. .. code::
  26. - [ ] Commit messages are in the standard format
  27. - [ ] Includes regression test
  28. - [ ] Includes documentation update (if applicable)
  29. - [ ] Is submitted against the correct branch
  30. - [ ] Has LGTM from at least one core developer
  31. Feature PRs:
  32. .. code::
  33. - [ ] Commit messages are in the standard format
  34. - [ ] Includes tests
  35. - [ ] Includes documentation
  36. - [ ] Is submitted against the correct branch
  37. - [ ] Has LGTM from at least two core developers
  38. Choosing a branch to submit to
  39. ------------------------------
  40. The following table assumes the latest stable release is 1.9.
  41. ============== ====================================
  42. Type of change Branch to submit against
  43. ============== ====================================
  44. Security fix 1.8 (Email security@elgg.org first!)
  45. Bug fix 1.9
  46. Deprecation 1.x
  47. Minor feature 1.x
  48. Major feature master
  49. Breaking master
  50. ============== ====================================
  51. The difference between minor and major feature is subjective and up to the core team.
  52. Commit message format
  53. ---------------------
  54. We require a particular format to allow releasing more often, and with improved changelogs and source history. Just
  55. follow these steps:
  56. 1. Start with the ``type`` by selecting the *last category which applies* from this list:
  57. * **docs** - *only* docs are being updated
  58. * **chore** - this include refactoring, code style changes, adding missing tests, Travis stuff, etc.
  59. * **perf** - the primary purpose is to improve performance
  60. * **fix** - this fixes a bug
  61. * **deprecate** - the change deprecates any part of the API
  62. * **feature** - this adds a new user-facing or developer feature
  63. * **security** - the change affects a security issue in any way. *Please do not push this commit to any public repo.* Instead contact security@elgg.org.
  64. E.g. if your commit refactors to fix a bug, it's still a "fix". If that bug is security-related, however, the type
  65. must be "security" and you should email security@elgg.org before proceeding. When in doubt, make your best guess
  66. and a reviewer will provide guidance.
  67. 2. In parenthesis, add the ``component``, a short string which describes the subsystem being changed.
  68. Some examples: "views", "i18n", "seo", "a11y", "cache", "db", "session", "router", "<plugin_name>".
  69. 3. Add a colon, a space, and a brief ``summary`` of the changes, which will appear in the changelog.
  70. No line may exceed 100 characters in length, so keep your summary concise.
  71. ================================================ ======================================================================================================
  72. Good summary Bad summary (problem)
  73. ================================================ ======================================================================================================
  74. page owners see their own owner blocks on pages bug fix (vague)
  75. bar view no longer dies if 'foo' not set updates views/default/bar.php so bar view no longer... (redundant info)
  76. narrows river layout to fit iPhone alters the river layout (vague)
  77. elgg_foo() handles arrays for $bar in elgg_foo() you can now pass an array for $bar and the function will... (move detail to description)
  78. removes link color from comments header in river fixes db so that... (redundant info)
  79. requires non-empty title when saving pages can save pages with no title (confusingly summarizes old behavior)
  80. ================================================ ======================================================================================================
  81. 4. (recommended) Skip a line and add a ``description`` of the changes. Include the motivation for making them, any info
  82. about back or forward compatibility, and any rationale of why the change had to be done a certain way. Example:
  83. We speed up the Remember Me table migration by using a single INSERT INTO ... SELECT query instead of row-by-row.
  84. This migration takes place during the upgrade to 1.9.
  85. Unless your change is trivial/obvious, a description is required.
  86. 5. If the commit resolves a GitHub issue, skip a line and add ``Fixes #`` followed by the issue number. E.g.
  87. ``Fixes #1234``. You can include multiple issues by separating with commas.
  88. GitHub will auto-close the issue when the commit is merged. If you just want to reference an issue, use
  89. ``Refs #`` instead.
  90. When done, your commit message will have the format:
  91. .. code::
  92. type(component): summary
  93. Optional body
  94. Details about the solution.
  95. Opportunity to call out as breaking change.
  96. Closes/Fixes/Refs #123, #456, #789
  97. Here is an example of a good commit message:
  98. .. code::
  99. perf(upgrade): speeds up migrating remember me codes
  100. We speed up the Remember Me table migration by using a single INSERT INTO ... SELECT query instead of row-by-row.
  101. This migration takes place during the upgrade to 1.9.
  102. Fixes #6204
  103. To validate commit messages locally, make sure ``.scripts/validate_commit_msg.php`` is executable, and make a copy
  104. or symlink to it in the directory ``.git/hooks/commit-msg``.
  105. .. code::
  106. chmod u+x .scripts/validate_commit_msg.php
  107. ln -s .scripts/validate_commit_msg.php .git/hooks/commit-msg/validate_commit_msg.php
  108. Rewriting commit messages
  109. -------------------------
  110. If your PR does not conform to the standard commit message format, we'll ask you to rewrite it.
  111. To edit just the last commit:
  112. 1. Amend the commit: ``git commit --amend`` (git opens the message in a text editor).
  113. 2. Change the message and save/exit the editor.
  114. 3. Force push your branch: ``git push -f your_remote your_branch`` (your PR with be updated).
  115. Otherwise you may need to perform an interactive rebase:
  116. 1. Rebase the last N commits: ``git rebase -i HEAD~N`` where N is a number.
  117. (Git will open the git-rebase-todo file for editing)
  118. 2. For the commits that need to change, change ``pick`` to ``r`` (for reword) and save/exit the editor.
  119. 3. Change the commit message(s), save/exit the editor (git will present a file for each commit that needs rewording).
  120. 4. ``git push -f your_remote your_branch`` to force push the branch (updating your PR).
  121. Testing
  122. =======
  123. Elgg has automated tests for both PHP and JavaScript functionality.
  124. All new contributions are required to come with appropriate tests.
  125. PHPUnit Tests
  126. -------------
  127. TODO
  128. Jasmine Tests
  129. -------------
  130. Test files must be named ``*Test.js`` and should go in either ``js/tests/`` or next
  131. to their source files in ``views/default/js``. Karma will automatically pick up
  132. on new ``*Test.js`` files and run those tests.
  133. Test boilerplate
  134. ----------------
  135. .. code:: js
  136. define(function(require) {
  137. var elgg = require('elgg');
  138. describe("This new test", function() {
  139. it("fails automatically", function() {
  140. expect(true).toBe(false);
  141. });
  142. });
  143. });
  144. Running the tests
  145. -----------------
  146. Elgg uses `Karma`_ with `Jasmine`_ to run JS unit tests.
  147. .. _Karma: http://karma-runner.github.io/0.8/index.html
  148. .. _Jasmine: http://pivotal.github.io/jasmine/
  149. You will need to have nodejs and npm installed.
  150. First install all the development dependencies:
  151. .. code::
  152. npm install
  153. Run through the tests just once and then quit:
  154. .. code::
  155. npm test
  156. You can also run tests continuously during development so they run on each save:
  157. .. code::
  158. karma start js/tests/karma.conf.js
  159. Coding best practices
  160. =====================
  161. Make your code easier to read, easier to maintain, and easier to debug.
  162. Consistent use of these guidelines means less guess work for developers,
  163. which means happier, more productive developers.
  164. General coding
  165. --------------
  166. Don't Repeat Yourself
  167. ^^^^^^^^^^^^^^^^^^^^^
  168. If you are copy-pasting code a significant amount of code, consider whether there's an opportunity to reduce
  169. duplication by introducing a function, an additional argument, a view, or a new component class.
  170. E.g. If you find views that are identical except for a single value, refactor into a single view
  171. that takes an option.
  172. **Note:** In a bugfix release, *some duplication is preferrable to refactoring*. Fix bugs in the simplest
  173. way possible and refactor to reduce duplication in the next minor release branch.
  174. Embrace SOLID and GRASP
  175. ^^^^^^^^^^^^^^^^^^^^^^^
  176. Use these `principles for OO design`__ to solve problems using loosely coupled
  177. components, and try to make all components and integration code testable.
  178. __ http://nikic.github.io/2011/12/27/Dont-be-STUPID-GRASP-SOLID.html
  179. Whitespace is free
  180. ^^^^^^^^^^^^^^^^^^
  181. Don't be afraid to use it to separate blocks of code.
  182. Use a single space to separate function params and string concatenation.
  183. Variable names
  184. ^^^^^^^^^^^^^^
  185. Use self-documenting variable names. ``$group_guids`` is better than ``$array``.
  186. Avoid double-negatives. Prefer ``$enable = true`` to ``$disable = false``.
  187. Interface names
  188. ^^^^^^^^^^^^^^^
  189. Use the pattern `Elgg\{Namespace}\{Name}`.
  190. Do not include an `I` prefix or an `Interface` suffix.
  191. We do not include any prefix or suffix so that we're encouraged to:
  192. * name implementation classes more descriptively (the "default" name is taken).
  193. * type-hint on interfaces, because that is the shortest, easiest thing to do.
  194. Functions
  195. ^^^^^^^^^
  196. Where possible, have functions/methods return a single type.
  197. Use empty values such as array(), "", or 0 to indicate no results.
  198. Be careful where valid return values (like ``"0"``) could be interpreted as empty.
  199. Functions not throwing an exception on error should return ``false`` upon failure.
  200. Functions returning only boolean should be prefaced with ``is_`` or ``has_``
  201. (eg, ``elgg_is_logged_in()``, ``elgg_has_access_to_entity()``).
  202. Ternary syntax
  203. ^^^^^^^^^^^^^^
  204. Acceptable only for single-line, non-embedded statements.
  205. Minimize complexity
  206. ^^^^^^^^^^^^^^^^^^^
  207. Minimize nested blocks and distinct execution paths through code. Use
  208. `Return Early`__ to reduce nesting levels and cognitive load when reading code.
  209. __ http://www.mrclay.org/2013/09/18/when-reasonable-return-early/
  210. Use comments effectively
  211. ^^^^^^^^^^^^^^^^^^^^^^^^
  212. Good comments describe the "why." Good code describes the "how." E.g.:
  213. Bad:
  214. .. code:: php
  215. // increment $i only when the entity is marked as active.
  216. foreach ($entities as $entity) {
  217. if ($entity->active) {
  218. $i++;
  219. }
  220. }
  221. Good:
  222. .. code:: php
  223. // find the next index for inserting a new active entity.
  224. foreach ($entities as $entity) {
  225. if ($entity->active) {
  226. $i++;
  227. }
  228. }
  229. Always include a comment if it's not obvious that something must be done in a certain way. Other
  230. developers looking at the code should be discouraged from refactoring in a way that would break the code.
  231. .. code:: php
  232. // Can't use empty()/boolean: "0" is a valid value
  233. if ($str === '') {
  234. register_error(elgg_echo('foo:string_cannot_be_empty'));
  235. forward(REFERER);
  236. }
  237. Commit effectively
  238. ^^^^^^^^^^^^^^^^^^
  239. * Err on the side of `atomic commits`__ which are highly focused on changing one aspect of the system.
  240. * Avoid mixing in unrelated changes or extensive whitespace changes. Commits with many changes are scary and
  241. make pull requests difficult to review.
  242. * Use visual git tools to craft `highly precise and readable diffs`__.
  243. __ http://en.wikipedia.org/wiki/Atomic_commit#Atomic_Commit_Convention
  244. __ http://www.mrclay.org/2014/02/14/gitx-for-cleaner-commits/
  245. Include tests
  246. ~~~~~~~~~~~~~
  247. When at all possible include unit tests for code you add or alter. We use:
  248. * PHPUnit for PHP unit tests.
  249. * SimpleTest for legacy PHP tests that require use of the database. Our long-term goal
  250. is to move all tests to PHPUnit.
  251. * Karma for JavaScript unit tests
  252. Naming tests
  253. ~~~~~~~~~~~~
  254. Break tests up by the behaviors you want to test and use names that describe the
  255. behavior. E.g.:
  256. * Not so good: One big method `testAdd()`.
  257. * Better: Methods `testAddingZeroChangesNothing` and `testAddingNegativeNumberSubtracts`
  258. Keep bugfixes simple
  259. ~~~~~~~~~~~~~~~~~~~~
  260. Avoid the temptation to refactor code for a bugfix release. Doing so tends to
  261. introduce regressions, breaking functionality in what should be a stable release.
  262. PHP guidelines
  263. --------------
  264. These are the required coding standards for Elgg core and all bundled plugins.
  265. Plugin developers are strongly encouraged to adopt these standards.
  266. Developers should first read the `PSR-2 Coding Standard Guide`__.
  267. __ https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md
  268. Elgg's standards extend PSR-2, but differ in the following ways:
  269. * Indent using one tab character, not spaces.
  270. * Opening braces for classes, methods, and functions must go on the same line.
  271. * If a line reaches over 100 characters, consider refactoring (e.g. introduce variables).
  272. * Compliance with `PSR-1`__ is encouraged, but not strictly required.
  273. __ https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-1-basic-coding-standard.md
  274. Documentation
  275. ^^^^^^^^^^^^^
  276. * Include PHPDoc comments on functions and classes (all methods; declared
  277. properties when appropriate), including types and descriptions of all
  278. parameters.
  279. * In lists of ``@param`` declarations, the beginnings of variable names and
  280. descriptions must line up.
  281. * Annotate classes, methods, properties, and functions with ``@access private``
  282. unless they are intended for public use, are already of limited visibility,
  283. or are within a class already marked as private.
  284. * Use ``//`` or ``/* */`` when commenting.
  285. * Use only ``//`` comments inside function/method bodies.
  286. Naming
  287. ^^^^^^
  288. * Use underscores to separate words in the names of functions, variables,
  289. and properties. Method names are camelCase.
  290. * Names of functions for public use must begin with ``elgg_``.
  291. * All other function names must begin with ``_elgg_``.
  292. * Name globals and constants in ``ALL_CAPS`` (``ACCESS_FRIENDS``, ``$CONFIG``).
  293. Miscellaneous
  294. ^^^^^^^^^^^^^
  295. For PHP requirements, see ``composer.json``.
  296. Do not use PHP shortcut tags ``<?`` or ``<%``.
  297. It is OK to use ``<?=`` since it is always enabled as of PHP 5.4.
  298. When creating strings with variables:
  299. * use double-quoted strings
  300. * wrap variables with braces only when necessary.
  301. Bad (hard to read, misuse of quotes and {}s):
  302. .. code:: php
  303. echo 'Hello, '.$name."! How is your {$time_of_day}?";
  304. Good:
  305. .. code:: php
  306. echo "Hello, $name! How is your $time_of_day?";
  307. Remove trailing whitespace at the end of lines. An easy way to do this before you commit is to run
  308. ``php .scripts/fix_style.php`` from the installation root.
  309. CSS guidelines
  310. --------------
  311. Use shorthand where possible
  312. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  313. Bad:
  314. .. code:: css
  315. background-color: #333333;
  316. background-image: url(...);
  317. background-repeat: repeat-x;
  318. background-position: left 10px;
  319. padding: 2px 9px 2px 9px;
  320. Good:
  321. .. code:: css
  322. background: #333 url(...) repeat-x left 10px;
  323. padding: 2px 9px;
  324. Use hyphens, not underscores
  325. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  326. Bad:
  327. .. code:: css
  328. .example_class {}
  329. Good:
  330. .. code:: css
  331. .example-class {}
  332. One property per line
  333. ^^^^^^^^^^^^^^^^^^^^^
  334. Bad:
  335. .. code:: css
  336. color: white;font-size: smaller;
  337. Good:
  338. .. code:: css
  339. color: white;
  340. font-size: smaller;
  341. Property declarations
  342. ^^^^^^^^^^^^^^^^^^^^^
  343. These should be spaced like so: `property: value;`
  344. Bad:
  345. .. code:: css
  346. color:value;
  347. color :value;
  348. color : value;
  349. Good:
  350. .. code:: css
  351. color: value;
  352. Vendor prefixes
  353. ^^^^^^^^^^^^^^^
  354. * Group vendor-prefixes for the same property together
  355. * Longest vendor-prefixed version first
  356. * Always include non-vendor-prefixed version
  357. * Put an extra newline between vendor-prefixed groups and other properties
  358. Bad:
  359. .. code:: css
  360. -moz-border-radius: 5px;
  361. border: 1px solid #999999;
  362. -webkit-border-radius: 5px;
  363. width: auto;
  364. Good:
  365. .. code:: css
  366. border: 1px solid #999999;
  367. -webkit-border-radius: 5px;
  368. -moz-border-radius: 5px;
  369. border-radius: 5px;
  370. width: auto;
  371. Group subproperties
  372. ^^^^^^^^^^^^^^^^^^^
  373. Bad:
  374. .. code:: css
  375. background-color: white;
  376. color: #0054A7;
  377. background-position: 2px -257px;
  378. Good:
  379. .. code:: css
  380. background-color: white;
  381. background-position: 2px -257px;
  382. color: #0054A7;
  383. Javascript guidelines
  384. ---------------------
  385. Same formatting standards as PHP apply.
  386. All functions should be in the ``elgg`` namespace.
  387. Function expressions should end with a semi-colon.
  388. .. code:: js
  389. elgg.ui.toggles = function(event) {
  390. event.preventDefault();
  391. $(target).slideToggle('medium');
  392. };
  393. Deprecating APIs
  394. ================
  395. Occasionally functions and classes must be deprecated in favor of newer
  396. replacements. Since 3rd party plugin authors rely on a consistent API,
  397. backward compatibility must be maintained, but will not be maintained
  398. indefinitely as plugin authors are expected to properly update their plugins.
  399. In order to maintain backward compatibility, deprecated APIs will follow
  400. these guidelines:
  401. * Minor version (1.x) that deprecates an API must include a wrapper
  402. function/class (or otherwise appropriate means) to maintain backward
  403. compatibility, including any bugs in the original function/class.
  404. This compatibility layer uses ``elgg_deprecated_notice('...', '1.11')``
  405. to log that the function is deprecated.
  406. * The next major revision (2.0) removes the compatibility layer.
  407. Any use of the deprecated API should be corrected before this.