search

monzo / response

Monzo's real-time incident response and reporting tool ⚡️
MIT License
1.53k stars 165 forks source link

Code formatting setup and CI #135

Closed danpalmer closed 5 years ago

danpalmer commented 5 years ago

As discussed in #133, this PR aims to introduce a basic code style, auto formatting setup, and linting for common Python coding errors.

The diff here is huge, but most of it comes from one commit that was almost entirely automated and can therefore probably be mostly ignored. I recommend reviewing the rest by commit. I've included some more background in commit messages where I can.

The changes here do fix some potential bugs, and I hope improve the consistency of some aspects of the code.

I'm open to discussion about the sorts of things being linted here, I realise this is quite a big change to the developer workflow and I'm keen to take into account existing style within Monzo for example. This setup is a simple version of what we use at Thread, and we find it to be effective in helping us to maintain a high code quality.

milesbxf commented 5 years ago

Thanks so much for this - unfortunately only saw it after we merged #134 so I'm afraid I've introduced conflicts already 😭

I'll make sure we don't merge anything else in the meantime - let me know once you've rebased and I'll prioritise looking at this and getting it merged!

danpalmer commented 5 years ago

@milesbxf Hey, I think I've addressed all the merge conflicts. I also found another flaky test and fixed that. If it's at all possible could you have a look at this today and perhaps cut another release to PyPI? I'm keen to work on some tooling using this at the weekend, and may want to contribute more changes here depending on how that goes. It would be great to be able to do this on top of these changes.

danpalmer commented 5 years ago

@milesbxf no problem, happy to do it, keen to get us using response internally too.

danpalmer commented 5 years ago

Thanks for the speedy review, much appreciated!