From 58b643bdefe2c9ba4d8f1ddc9f15868288b48685 Mon Sep 17 00:00:00 2001 From: Laurent VAYLET Date: Tue, 25 Oct 2022 10:52:07 +0200 Subject: [PATCH 1/2] add details on development workflow --- CONTRIBUTING.md | 78 +++++++++++++++++++++++++++++++------------------ 1 file changed, 50 insertions(+), 28 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ec27d753..e4afe102 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,43 +20,65 @@ This project follows [Google's Open Source Community Guidelines](https://opensou ### Development environment -To prepare for development, you need to fork this repository and work on your own branch so that you can later submit your changes as a GitHub Pull Request. +1. To prepare for development, you need to fork this repository and work on your own branch so that you can later submit your changes as a GitHub Pull Request. -Once you have forked the repo on GitHub, clone it locally and install the `slo-generator` in a Python virtual environment: +1. Once you have forked the repo on GitHub, clone it locally and create a Python virtual environment to work in: -```sh -git clone github.com/google/slo-generator -cd slo-generator -python3 -m venv venv/ -source venv/bin/activate -``` + ```sh + git clone github.com/google/slo-generator + cd slo-generator + python3 -m venv venv/ + source venv/bin/activate + ``` -Then install `slo-generator` locally in development mode, with all the extra packages as well as pre-commit hooks in order to speed up and simplify the development workflow: +1. Install `slo-generator` locally in development mode, with all the extra packages as well as pre-commit hooks in order to speed up and simplify the development workflow: -```sh -make develop -``` + ```sh + make develop + ``` -Finally, feel free to start making changes. +1. Start making changes. -Note that [pre-commit](https://pre-commit.com/) hooks are installed during `make develop` and automatically executed by `git`. These checks are responsible for making sure your changes are linted and well-formatted, in order to match the rest of the codebase and simplify the code reviews. You will be warned after issuing `git commit` if at least one of the staged files does not comply. You can also run the checks manually on the staged files at any time with: +1. Commit your changes locally, in small chunks with meaningful commit messages to simplify the code reviewing process later on. -```sh -$ pre-commit run -trim trailing whitespace.................................................Passed -fix end of files.........................................................Passed -check yaml...............................................................Passed -check for added large files..............................................Passed -autoflake................................................................Passed -flake8...................................................................Passed -black....................................................................Passed -isort....................................................................Passed -pylint...................................................................Passed -``` + Note that [pre-commit](https://pre-commit.com/) hooks are installed during `make develop` and automatically executed by `git` on every `git commit` operation. These checks are responsible for making sure your changes are linted and well-formatted, in order to match the rest of the codebase and simplify the code reviews. You will be warned after issuing `git commit` if at least one of the staged files does not comply. You can also run the checks manually on the staged files at any time with: + + ```sh + $ pre-commit run + trim trailing whitespace.................................................Passed + fix end of files.........................................................Passed + check yaml...............................................................Passed + check for added large files..............................................Passed + autoflake................................................................Passed + flake8...................................................................Passed + black....................................................................Passed + isort....................................................................Passed + pylint...................................................................Passed + ``` -If any error is reported, your commit gets canceled. At this point, run `make format` to automatically reformat the code with [`isort`](https://github.com/PyCQA/isort) and [`black`](https://github.com/psf/black). Also make sure to fix any errors returned by linters or static analyzers such as [`flake8`](https://flake8.pycqa.org/en/latest/), [`pylint`](https://pylint.pycqa.org/en/latest/), [`mypy`](http://mypy-lang.org/) or [`pytype`](https://github.com/google/pytype). Then commit again, rinse and repeat. + If any error is reported, your commit gets canceled and you can start working on fixing the issues. If a formatting error gets reported, run `make format` to automatically organize the imports and reformat the code with [`isort`](https://github.com/PyCQA/isort) and [`black`](https://github.com/psf/black). Also make sure to fix any errors returned by linters or static analyzers such as [`flake8`](https://flake8.pycqa.org/en/latest/), [`pylint`](https://pylint.pycqa.org/en/latest/), [`mypy`](http://mypy-lang.org/) or [`pytype`](https://github.com/google/pytype). Then try `git commit`-ting your changes again. + + Ignoring these pre-commit warnings is not recommended. The Continuous Integration (CI) pipelines will run the exact same checks (and more!) when your commits get pushed. The checks will fail there and prevent you from merging your changes to the `master` branch anyway. So fail fast, fail early, fail often and fix as many errors as possible on your local development machine. Code reviews will be more enjoyable for everyone! + +1. Push your changes when all the pre-commit checks complete successfully. + +1. Create a Pull Request (PR) and ask for a review. + +***Notes:*** + +- IDEs such as Visual Studio Code and PyCharm can help with linting and reformatting. For example, Visual Studio Code can be configured to run `isort` and `black` automatically on every file save. Just add these lines to your `settings.json` file, as documented in [this article](https://cereblanco.medium.com/setup-black-and-isort-in-vscode-514804590bf9): + + ```json + "editor.formatOnSave": true, + "python.formatting.provider": "black", + "[python]": { + "editor.codeActionsOnSave": { + "source.organizeImports": true // isort + } + }, + ``` -Ignoring these pre-commit warnings is not recommended. The Continuous Integration (CI) pipelines will run the exact same checks (and more!) when your commits get pushed. The checks will fail there and prevent you from merging your changes to the `master` branch anyway. So fail fast, fail early, fail often and fix as many errors as possible on your local development machine. Code reviews will be more enjoyable for everyone! +- [`pyenv`](https://github.com/pyenv/pyenv) can be used to easily switch between multiple versions of Python. For example, you might want to create multiple virtual environments like `venv3.9.14` and `venv3.10.7` to confirm a syntax or feature is supported by all Python versions, while keeping the environments isolated from each other. ### Testing environment From dcdd16d2085a6b8c880d7fcf7caff3c74738aa18 Mon Sep 17 00:00:00 2001 From: Laurent VAYLET Date: Tue, 25 Oct 2022 10:53:33 +0200 Subject: [PATCH 2/2] fix markdown style violations --- CONTRIBUTING.md | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e4afe102..c475ada8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -105,22 +105,22 @@ The `slo-generator` tool is designed to be modular as it moves forward. Users, c To add a new backend, one must: -* Add a new file named `slo-generator/backends/.py` -* Write a new Python class called `Backend` (CamlCase) -* Test it with a sample config -* Add some unit tests -* Make sure all tests pass -* Submit a PR +- Add a new file named `slo-generator/backends/.py` +- Write a new Python class called `Backend` (CamlCase) +- Test it with a sample config +- Add some unit tests +- Make sure all tests pass +- Submit a PR ***Example with a fake Cat backend:*** -* Add a new backend file: +- Add a new backend file: ```sh touch slo-generator/backends/cat.py ``` -* Fill the content of `cat.py`: +- Fill the content of `cat.py`: ```python from provider import CatClient @@ -168,7 +168,7 @@ To add a new backend, one must: return my_sli_value ``` -* Write a sample SLO configs (`slo_cat_test_slo_ratio.yaml`): +- Write a sample SLO configs (`slo_cat_test_slo_ratio.yaml`): ```yaml service_name: cat @@ -184,18 +184,18 @@ To add a new backend, one must: query_valid: avg:system.disk.used{*}.rollup(avg, {window}) ``` -* Run a live test with the SLO generator: +- Run a live test with the SLO generator: ```sh slo-generator -f slo_cat_test_slo_ratio.yaml -b samples/error_budget_target.yaml ``` -* Create a directory `samples/` for your backend samples. -* Add some YAML samples to show how to write SLO configs for your backend. Samples should be named `slo___.yaml`. -* Add a unit test: in the `tests/unit/test_compute.py`, simply add a method called `test_compute_`. Take the other backends an example when +- Create a directory `samples/` for your backend samples. +- Add some YAML samples to show how to write SLO configs for your backend. Samples should be named `slo___.yaml`. +- Add a unit test: in the `tests/unit/test_compute.py`, simply add a method called `test_compute_`. Take the other backends an example when writing the test. -* Add documentation for your backend / exporter in a new file named `docs/providers/cat.md`. -* Make sure all tests pass -* Submit a PR +- Add documentation for your backend / exporter in a new file named `docs/providers/cat.md`. +- Make sure all tests pass +- Submit a PR The steps above are similar for adding a new exporter, but the exporter code will go to the `exporters/` directory and the unit test will be named `test_export_`.