fdrozdowski
commented
4 years ago ➤ Grant Magdanz commented:
The description above isn't up to date:
The precommit action has been put in place but common, libs, and platform have not been linted or formatted. tools is being linted, but not formatted.
We still want to switch to black from yapf for a couple reasons. First, yapf is not compatible with the flake8 linter we use. Second, black is what the QA gate and other python projects at Delphix use. Third, black is extremely strict and easy to use with yapf is confusing and sometimes not even deterministic.
All the python code in the SDK should be formatted with black, linted, and both should be enforced in the precommit action. This should be done after the cutover. Otherwise there will be a lot of merge conflicts.
The one downside to black is that it requires Python 3 to run. It can still format Python 2.7, but needs to be ran with Python 3. Murali has context here, but the plan is to put that burden on developers. They can create a Python 3 virtual environment to run black if they want (this is what we should probably document). They can also just install it in user mode since it's a tool and not a library.
To be explicit, we do not plan to have a helper script checked in to help with formatting. This is because black is extremely easy to run manually, it's important for developers to know the tools their using, and helper scripts are notoriously difficult to maintain.
common, libs, and platform have not been formatted or linted. They need to be before the CI/CD pipeline is put in place.
"tools" is also formatted using yapf. We are moving to black for formatting to be consistent. It needs to be formatted correctly by black.
Formatting and linting should also be enforced in GitHub Actions as part of this task.
┆Issue is synchronized with this Jira Story by Unito