There are several steps to follow when you want to submit your change for review.
Thoroughly test your code to ensure that everything works as expected. Your change should not break what is already working.
Execute all automatic tests covering a system you develop and make sure all of them pass.
Whether you work on frontend or contribute to backend part of the system, your code should be verified for quality.
We use pylint as a code quality checker for python. Run pylint over the code you've been working on every time you are going to commit your changes. Configuration for pylint (usually .pylintrc) can be project specific, so ask your manager for details.
Installation
$ pip install pylint
Generate configuration file
The following command will generate and save pylintrc file to the current directory. Options appearing before
--generate-rcfile will be included to configuration file:
$ pylint --disable=C0123 --generate-rcfile > .pylintrc
Usage
Simply point pylint to a file or directory you want to check:
$ pylint path/to/your/file/or/dir
Disable specific warnings
You can avoid specific messages using --disable option:
$ pylint --disable=C0123 path/to/your/file/or/dir
eslint is a tool of our choice for problem detection in JavaScript code. We prefer Google style as a style guide and use Vue plugin when work on a project based on this framework.
Installation
$ npm install --save-dev eslint eslint-plugin-vue@next
Generate configuration file
$ npx eslint --init # Choose Google style when asked
IDE integration
eslint is easily integrated with many IDEs including VS Code. For this go to "Manage (gear icon at the bottom left) -> Estensions" and type eslint. To enable eslint as a formatter in VS Code go to "Manage -> Settings" and search for "eslint formatter".
Usage
If you prefer to run eslint in command line, you can do this as follows:
$ npx eslint ./src/
Properly describe a problem you solve. If you feel that only commit title does not give enough information about your change, add body. Here you can put technical details, bug reproduction routes, crash/exception messages, quantitative data on performance improvements, optimizations, etc.
It's a good idea to provide a URL to corresponding ticket in issue tracking system or StackOverflow question that helped you to solve a problem. However, try to make your explanation understandable without referring to external resources.
Separate each logical change into a separate patch. For example, if your changes include both bug fixes and API update for a same resource, separate those changes into two or more commits. Make a separate commit for coding style fixes. Clean git history makes review easier and facilitates using tools for detecting bugs, such as git-bisect .
Always rebase on top of the branch where you want your changes to be merged before push. The purpose of this is:
fix conflicts at early stage;
allows to test your work against recent changes in a target branch.
Squash related commits which represent one logical change into one to keep git history clean:
$ git rebase -i <base_commit_id> # then mark commits to be squashed
When your change is ready, push it and create a pull request. If reviewer ask you for changes, make necessary fixes, rebase on top, push-force and ask to review again.
IMPORTANT: push-force is allowed only to your private branch; never do push-force to a branch shared with other people.
Finally, when your change is approved, it can be merged.
Great materials to read:
How to Write a Git Commit Message