search

FelixSchwarz / mjml-python

Python implementation for MJML - a framework that makes responsive-email easy
MIT License
73 stars 16 forks source link

enable ruff with a somewhat minimal configuration #32

Closed FelixSchwarz closed 1 year ago

FelixSchwarz commented 1 year ago

As @caseyjhol noticed the code currently reflects a lot of my personal preferences. In order to get some consistency I thought about adding ruff via pre-commit with a somewhat minimal configuration.

Currently this is not enabled in CI but I just want to know if this is a good idea or not.

@caseyjhol What is your opinion on this?

caseyjhol commented 1 year ago

I haven't had a chance to fully look this over, but I think something like this would be great and definitely improve DX.

caseyjhol commented 1 year ago

I'd also propose we always use function names that match MJML upstream (where possible). e.g. I think we should always use self.getAttribute (we also use self.get_attr in many places). This makes converting the upstream code to Python a little easier.

FelixSchwarz commented 1 year ago

In the beginning I also thought that matching the function name 1:1 would be the best option. After working a bit in the mjml source code I decided that camel case is just too painful as it looks too alien in Python. In the end if all camel-case names are converted into snake-case the mental mapping should be pretty fast as well (e.g. fooBar() → foo_bar()).

With .get_attr() I went down a slippery slope: I found that "in that special case" .get_attribute() was pretty long and a shorter method name makes it easier to see what output data is created instead. I could see that we should .get_attribute() but I really would like to use snake-case instead of camel-case.