CivicTechJobs icon indicating copy to clipboard operation
CivicTechJobs copied to clipboard

Enable linting for python code

Open Aveline-art opened this issue 2 years ago • 2 comments

Overview

As a developer, we should make sure that our code is maintainable. For this ticket, let's start a discussion on whether linting python code is desirable.

Action Items

  • [x] Discuss with the team on whether adding a python linter is desirable.
  • [ ] Enable the python linter for super-linter if needed.

Resources/Instructions

Resources

This has never been up for consideration previously, since vscode did not have native support for python linting, meaning that we needed to download them as libraries, which would impact our build size. Now two things have changed:

  1. We are using poetry now, so we can specify optional dev dependencies.
  2. VSCode now has an extension for black and isort. This means that we do not to fangle with any external libraries.

Aveline-art avatar Jun 03 '23 03:06 Aveline-art

Related issue: #587

irais-valenzuela avatar Sep 27 '24 01:09 irais-valenzuela

Note: This docker code from Ida's PR #587, that came from the main branch (written by Matt Pereira over a year ago) would enable the python linter in dev mode:

 linter:
    profiles: ["lint"]
    build:  
      context: .
      dockerfile: ./dev/linter.dockerfile
    container_name: linter
    environment:
      - PRE_COMMIT_HOME=${HOME}/.cache/pre-commit
    user: ${UID}:${GID}
    env_file:
      - ./dev/linter.env
    volumes:
      - .:/src:rw
      - ${HOME}/.cache:${HOME}/.cache:rw

I need to investigate this further.

  • [ ] Test the dev python linter container
  • [ ] Upgrade it or make changes if necessary
  • [ ] Finally, document changes in mkdocs

The benefit of having a docker container for linting the backend is that no one will have to install the specific version of python or poetry or all the vscode python linter extensions, which can be overly complicated for new devs.

Things to keep in mind:

  • this implementation enables the pre-commit config + hooks through the docker container. Not sure how I feel about this yet.
  • It also has a separate linter.env file specifically for the pre-commit
  • At the moment, the installation instructions in the mkdocs include instructions on how to start the linter container detailed here. This needs to be changed, as of now we are not using it (at least in the develop branch)
  • I believe there is also a github action for linting the backend before merging to main.. Need to look into this. It is likely using the same config Matt Pereira made here.
  • There exists a linters/ folder in the .github/ folder. I need to investigate what this is for.

Idea

  • separate codebase into v1 and v2 branches first so there are no conflicts between what we have in develop now and the linter config that was in main before

Resources

  • https://hackforla.github.io/CivicTechJobs/developer/installation/
  • https://hackforla.github.io/CivicTechJobs/developer/devops/

LoTerence avatar Oct 08 '24 05:10 LoTerence

Next step: Add python and eslint automations to CI/CD pipeline #611

LoTerence avatar Nov 07 '24 23:11 LoTerence