cabotage / cabotage-app

MIT License
28 stars 8 forks source link

add: ci tooling for security, type, lint checks #83

Closed ayhamthemayhem closed 4 months ago

ayhamthemayhem commented 4 months ago

Extend existing GitHub actions workflow with the recommendations made by this security audit

ewdurbin commented 4 months ago

This conflicts with #81, which removes tox as a dependency locally and simplifies dev to just requiring docker/docker-compose.

Would you be opposed to seing #81 merged and rebasing on that?

ayhamthemayhem commented 4 months ago

All good I can adjust after #81 gets merged :)

ewdurbin commented 4 months ago

👍🏼, I've merged #81

One note, is to add the new deps to requirements/dev.in rather than requirements/base.in. I noticed that this PR also upgraded all dependencies which is probably not ideal in one-shot.

ayhamthemayhem commented 4 months ago

Good to know ! I noticed that the deps where updated also this is the first time for me working on a python project and there still so much to learn here :).

Hopefully I can adjust and land this PR by early next week.

ewdurbin commented 4 months ago

Cool, only other note is that both mypy and bandit support configuration via pyproject.toml:

I think we should prefer that over having a bunch of various config files added.

ewdurbin commented 4 months ago

Hi @ayhamthemayhem! Can you address the comments above? I don't think we want to upgrade to Python 3.12 as part of this, and it appears that there is some vestigial references to tox after #81 was merged.

ayhamthemayhem commented 4 months ago

Python update was part of me trying to figure why there is a miss match in the dep hashes when I run the CI in our fork and that is caused by some deps being imported in dev.txt and base.txt by different packages but I will address it in my fork and I will bring it back to 3.11. If you have some insight on how to resolve this issue please let me know :)

ewdurbin commented 4 months ago

Thanks @ayhamthemayhem! One note for future PRs. We've enabled "Squash and merge" for pull requests so that there's no need to continually re-base and force-push to have things be a single commit. The entire changeset from the PR will be merged to main as a single commit.

Force pushing makes review more difficult than it needs to be as changes are pushed, leaving comments made during review inscrutable.