User Tools
Differences
This shows you the differences between two versions of the page.
| Both sides previous revision Previous revision Next revision | Previous revision | ||
|
en:devel:developmentguidelines [2017/10/08 12:22] joebordes [Code Formatting and Validation tools] |
en:devel:developmentguidelines [2021/01/06 12:09] (current) joebordes [Security] |
||
|---|---|---|---|
| Line 13: | Line 13: | ||
| The best place to contact the developer community is [[https://gitter.im/corebos/discuss|on our gitter chat group]]. | The best place to contact the developer community is [[https://gitter.im/corebos/discuss|on our gitter chat group]]. | ||
| - | The [[http://blog.corebos.org:8080|blog]] is mostly developer oriented and there is a lot of information here on the documentation site. | + | The [[https://blog.corebos.org|blog]] is mostly developer oriented and there is a lot of information here on the documentation site. |
| You can [[http://discussions.corebos.org/|ask in the forum]] also, but don't hesitate to get in touch, we are a really **friendly and helpful community**. | You can [[http://discussions.corebos.org/|ask in the forum]] also, but don't hesitate to get in touch, we are a really **friendly and helpful community**. | ||
| Line 63: | Line 63: | ||
| * Strings should be enclosed in SINGLE QUOTES where possible. | * Strings should be enclosed in SINGLE QUOTES where possible. | ||
| + | The [[https://docs.nextcloud.com/server/14/developer_manual/general/codingguidelines.html|Nextcloud coding style is a very sane set of rules also]] | ||
| Line 82: | Line 82: | ||
| * use vtlib_purify on all incoming and outgoing information | * use vtlib_purify on all incoming and outgoing information | ||
| * user Vtiger_Request to construct URLs | * user Vtiger_Request to construct URLs | ||
| + | |||
| + | [[https://docs.nextcloud.com/server/14/developer_manual/general/codingguidelines.html|Read the Nextcloud recommendations]] | ||
| ===== Commit Guidelines ===== | ===== Commit Guidelines ===== | ||
| ==== BEFORE Committing ==== | ==== BEFORE Committing ==== | ||
| - | This is a list of things that you MUST do before executing a commit: | + | This is a [[https://github.com/tsolucio/corebos/issues/911#issue-719191770|list of things that you MUST do before executing a commit]]: |
| - | * Check if you must separate the code changes in various commits. When I program I usually make some cosmetic changes or modifications that are not related to the requirement I am implementing. You can use the "-p" directive to split unrelated changes creating concise and cohesive commit changes. | + | * //git diff// check all the files that have been modified, read the diff, and mentally review the change. eliminate debug messages and passwords. |
| * Check for debug messages you may have left behind. Before I launch a commit I usually execute these commands: <code>git diff {files} | grep dump | * Check for debug messages you may have left behind. Before I launch a commit I usually execute these commands: <code>git diff {files} | grep dump | ||
| git diff {files} | grep fatal | git diff {files} | grep fatal | ||
| Line 94: | Line 96: | ||
| </code> | </code> | ||
| * Check for php errors <code>for f in {diff file list}; do php -l $f; done;</code> | * Check for php errors <code>for f in {diff file list}; do php -l $f; done;</code> | ||
| - | * Ideally you should run a lint process on both php and javascript. I haven't gotten to this yet but it will happen sooner than later and I will add it to the Pull Request acceptance process. | + | * //git status// any new/untracked files to add? |
| + | * Any strings that should be translated? | ||
| + | * If you alter an existing function, did you check if all the existing references to that function will still behave the same way? | ||
| + | * If you created new code, did you try and use existing functionality as much as possible? | ||
| + | * [[en:devel:developmentguidelines#code_formatting_and_validation_tools|code formatting]] | ||
| + | * Ideally you should run a lint process on both PHP and javascript. Please look below for the tools we use and how to execute them. I will add it to the Pull Request acceptance process at some point. | ||
| + | * see the checkfile executable below | ||
| + | * Did you execute the unit tests and/or e2e tests? | ||
| + | * //git add -p// to separate commits into semantically meaningful chunks. Check if you must separate the code changes in various commits. When I program I usually make some cosmetic changes or modifications that are not related to the requirement I am implementing. You can use the "-p" directive to split unrelated changes creating concise and cohesive commit changes. | ||
| + | * dedicate a moment to [[en:devel:developmentguidelines#commit_guidelines|think about the commit message]] | ||
| + | * Any documentation that should be added to the wiki? | ||
| {{page>en:devel:commitguidelines&nofooter&nouser&nodate&noeditbtn}} | {{page>en:devel:commitguidelines&nofooter&nouser&nodate&noeditbtn}} | ||
| + | |||
| + | |||
| + | ==== Special Committs ==== | ||
| + | |||
| + | === Service Worker Commit === | ||
| + | |||
| + | These are the steps to update the service worker: | ||
| + | |||
| + | <code> | ||
| + | fold service-worker.js > sold | ||
| + | include/sw-precache/regen_swprecache | ||
| + | fold service-worker.js > snew | ||
| + | meld sold snew | ||
| + | # make sure the update is about the files you know have changed | ||
| + | # the typical error here is for the update to include some javascript or css code you have not committed yet | ||
| + | rm sold snew | ||
| + | </code> | ||
| + | |||
| + | **Always** commit the service worker update in it's own commit. Do not mix it with any other changes. This way we can safely ignore all those commits when searching, or easily locate changes when needed. | ||
| + | |||
| ===== Code Formatting and Validation tools ===== | ===== Code Formatting and Validation tools ===== | ||
| Line 104: | Line 135: | ||
| * [[https://github.com/squizlabs/PHP_CodeSniffer|PHP Code Sniffer]] for PHP | * [[https://github.com/squizlabs/PHP_CodeSniffer|PHP Code Sniffer]] for PHP | ||
| * [[https://eslint.org|ESLint]] for Javascript | * [[https://eslint.org|ESLint]] for Javascript | ||
| + | * [[https://phpmd.org/|PHP Mess Detector]] | ||
| You can find rulesets for the above guidelines in the build/cbSR directory. | You can find rulesets for the above guidelines in the build/cbSR directory. | ||
| Line 113: | Line 145: | ||
| </code> | </code> | ||
| - | <WRAP center round info 60%> | + | You could also execute this command on javascript files, but eslint is better: |
| - | You could do this also with javascript files. | + | |
| - | </WRAP> | + | |
| - | + | ||
| - | To validate a javascript file you would execute: | + | |
| <code> | <code> | ||
| Line 126: | Line 154: | ||
| You can use phpcbf and the eslint --fix to get changes applied automatically | You can use phpcbf and the eslint --fix to get changes applied automatically | ||
| </WRAP> | </WRAP> | ||
| + | |||
| + | I actually bundle phpcs and phpmd in one file (checkfile) that I use to validate my php files: | ||
| + | |||
| + | <code> | ||
| + | echo ====== | ||
| + | echo $1 | ||
| + | echo ====== | ||
| + | phpcs.phar --standard=/var/www/coreBOSwork/build/cbSR $1 | ||
| + | phpmd.phar $1 text unusedcode | ||
| + | php -l $1 | ||
| + | </code> | ||
| + | ===== Some important management recommendations ===== | ||
| + | * Custom modules: | ||
| + | * Each module must have it's own repository | ||
| + | * You must try to keep the modules' repository in sync with the installs where it is in production | ||
| + | * Since that is a bit difficult what I do is update them when I need to install them somewhere | ||
| + | * [[en:extensions:extensions|Register your module in the extensions section]] so it can be found in the future. You can [[en:devel:helperscripts#composer2readme_and_module2wiki|use the helper scripts]]. | ||
| + | * There **MUST NOT** be any unversioned changes in production. This is MANDATORY | ||
| + | * For EVERY change or customization that you have to do in coreBOS, challenge yourself to find a way to make that change without modifying one line of base code that you can find in the github repository. You will be surprised how much can be done, you will learn a lot and have much more fun :-) | ||
| + | |||
| ===== References and further reading ===== | ===== References and further reading ===== | ||
| Line 138: | Line 186: | ||
| * [[https://github.com/squizlabs/PHP_CodeSniffer|PHP Code Sniffer]] | * [[https://github.com/squizlabs/PHP_CodeSniffer|PHP Code Sniffer]] | ||
| * [[https://eslint.org/|ESLint]] | * [[https://eslint.org/|ESLint]] | ||
| + | * [[https://github.com/rwaldron/idiomatic.js|Writing Consistent, Idiomatic JavaScript]] | ||
| + | * [[http://jstherightway.org/|JavaScript, The Right Way]] | ||
