cisagov / skeleton-ansible-role

A skeleton project for quickly getting a new cisagov Ansible role started.
Creative Commons Zero v1.0 Universal
7 stars 6 forks source link

Install `yamllint` and the Python libraries used for code linting and formatting for developers #202

Closed jsf9k closed 4 months ago

jsf9k commented 5 months ago

🗣 Description

This pull request installs yamllint and the Python libraries we use for code linting and formatting.

💭 Motivation and context

We can expect developers to install non-Python tools locally via their system's package manager, but the Python-based tools are a different case. We provide requirements*.txt files to encourage developers to set up a local Python virtual environment (venv), and many editors will make use of that venv when performing syntax checking and the like. It therefore makes sense to go ahead and include in requirements-dev.txt any Python tools that we use for formatting and linting of code.

(Note that installing mypy would be even more useful if pytest-testinfra were typed, but that is not yet the case. See pytest-dev/pytest-testinfra#619 for more details about that.)

I noticed that my editor was complaining about trying to use mypy to provide more information but not finding it available, and I verified that installing it in my venv made that complaint go away. That's what started this whole PR effort.

🧪 Testing

All automated tests pass. I noticed that my editor was complaining about trying to use mypy and yamllint to provide more information but not finding them available, and I verified that installing them in my venv made the complaints go away.

✅ Pre-approval checklist

jsf9k commented 4 months ago

Now that I've created this PR, I think that cisagov/skeleton-generic might be a better home for these changes. What do you think @cisagov/vm-dev?

jsf9k commented 4 months ago

I don't really understand the heavy overlap with our pre-commit configuration here. All of these tools (exceptingmypy) can just as effectively be run using pre-commit run --all-files, more specifically pre-commit run <hook name> --all-files, or with pre-commit installed as a git hook it will be run on commit. mypy is a separate case because of how it interacts when living in a Python venv, but everything else would potentially conflict with our pre-commit configuration since there is no version pinning happening here.

You raise a good point. In the interest of documenting the intent behind this PR, these are the types of problems I was trying to solve:

  1. I'm editing a Python file and my editor wants to provide extra typing information, but it can't because mypy isn't installed in the local venv (which my editor is using).
  2. I'm editing a Python file and my editor wants to provide flake8 warning and error information, but it can't because flake8 isn't installed in the local venv (which my editor is using).
  3. I'm editing a YML file and my editor wants to do some syntax checking with yamllint, but it can't because yamllint isn't installed in the local venv (which my editor is using).
  4. I'm editing a Python file and I know that black will want to make changes. To save time I'd like to go ahead and run black against the file without leaving my editor. I can't do that because black isn't installed in the local venv (which my editor is using).

I will handle these dependencies locally. I thought this addition might be useful for others, but I can see that it would cause headaches.